Skip to content

Commit 623d2b5

Browse files
oceanocean
committed
tests: Fix various issues in tests and formatting
1 parent 632c3ba commit 623d2b5

8 files changed

Lines changed: 267 additions & 171 deletions

File tree

‎AGENTS.md‎

Lines changed: 5 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ Named parameters in transactions:
578578
- **Prevention**: Prevents SQL injection attacks through proper parameter binding
579579

580580
**Backward Compatibility:**
581-
Positional parameters (`?`) still work unchanged. Mix positional and named parameters carefully - SQLite applies them in parameter-order:
581+
Positional parameters (`?`) still work unchanged:
582582

583583
```elixir
584584
# Positional parameters still work
@@ -589,10 +589,12 @@ Positional parameters (`?`) still work unchanged. Mix positional and named param
589589
state
590590
)
591591

592-
# Named parameters can coexist with positional in same Elixir codebase
593-
# (but not in the same query - SQLite doesn't allow mixing syntaxes)
592+
# Named and positional can coexist in separate queries within the same codebase
594593
```
595594

595+
**Avoiding Mixed Syntax:**
596+
While SQLite technically permits mixing positional (`?`) and named (`:name`) parameters in a single statement, this is discouraged. Named parameters receive implicit numeric indices which can conflict with positional parameters, leading to unexpected binding order. This adapter's map-based approach naturally avoids this issue—pass a list for positional queries, or a map for named queries, but don't mix within a single statement.
597+
596598
#### How Statement Caching Works
597599

598600
Prepared statements are now cached internally after preparation:
@@ -1603,100 +1605,6 @@ end
16031605
CREATE TABLE sessions (...) RANDOM ROWID
16041606
```
16051607

1606-
#### STRICT Tables (Type Enforcement)
1607-
1608-
STRICT tables enforce strict type checking - columns must be one of the allowed SQLite types. This prevents accidental type mismatches and data corruption:
1609-
1610-
```elixir
1611-
# Create a STRICT table for type safety
1612-
defmodule MyApp.Repo.Migrations.CreateUsers do
1613-
use Ecto.Migration
1614-
1615-
def change do
1616-
create table(:users, strict: true) do
1617-
add :id, :integer, primary_key: true
1618-
add :name, :string, null: false
1619-
add :email, :string, null: false
1620-
add :age, :integer
1621-
add :balance, :float, default: 0.0
1622-
add :avatar, :binary
1623-
add :is_active, :boolean, default: true
1624-
1625-
timestamps()
1626-
end
1627-
1628-
create unique_index(:users, [:email])
1629-
end
1630-
end
1631-
```
1632-
1633-
**Benefits:**
1634-
- **Type Safety**: Enforces that columns only accept their declared types (TEXT, INTEGER, REAL, BLOB, NULL)
1635-
- **Data Integrity**: Prevents accidental type coercion that could lead to bugs
1636-
- **Better Errors**: Clear error messages when incorrect types are inserted
1637-
- **Performance**: Can enable better query optimisation by knowing exact column types
1638-
1639-
**Allowed Types in STRICT Tables:**
1640-
- `INT`, `INTEGER` - Integer values only
1641-
- `TEXT` - Text values only
1642-
- `BLOB` - Binary data only
1643-
- `REAL` - Floating-point values only
1644-
- `NULL` - NULL values only (rarely used)
1645-
1646-
**Usage Examples:**
1647-
1648-
```elixir
1649-
# STRICT table with various types
1650-
create table(:products, strict: true) do
1651-
add :sku, :string, null: false # Must be TEXT
1652-
add :name, :string, null: false # Must be TEXT
1653-
add :quantity, :integer, default: 0 # Must be INTEGER
1654-
add :price, :float, null: false # Must be REAL
1655-
add :description, :text # Must be TEXT
1656-
add :image_data, :binary # Must be BLOB
1657-
add :published_at, :utc_datetime # Stored as TEXT (ISO8601 format)
1658-
timestamps()
1659-
end
1660-
1661-
# Combining STRICT with RANDOM ROWID
1662-
create table(:api_keys, options: [strict: true, random_rowid: true]) do
1663-
add :user_id, references(:users, on_delete: :delete_all) # INTEGER
1664-
add :key, :string, null: false # TEXT
1665-
add :secret, :string, null: false # TEXT
1666-
add :last_used_at, :utc_datetime # TEXT
1667-
timestamps()
1668-
end
1669-
```
1670-
1671-
**Restrictions:**
1672-
- STRICT is a libSQL/SQLite 3.37+ extension (not available in older versions)
1673-
- Type affinity is enforced: generic types like `TEXT(50)` or `DATE` are not allowed
1674-
- Dynamic type changes (e.g., storing integers in TEXT columns) will fail with type errors
1675-
- Standard SQLite does not support STRICT tables
1676-
1677-
**SQL Output:**
1678-
```sql
1679-
CREATE TABLE users (
1680-
id INTEGER PRIMARY KEY,
1681-
name TEXT NOT NULL,
1682-
email TEXT NOT NULL,
1683-
age INTEGER,
1684-
balance REAL DEFAULT 0.0,
1685-
avatar BLOB,
1686-
is_active INTEGER DEFAULT 1,
1687-
inserted_at TEXT,
1688-
updated_at TEXT
1689-
) STRICT
1690-
```
1691-
1692-
**Error Example:**
1693-
```elixir
1694-
# This will fail on a STRICT table:
1695-
Repo.query!("INSERT INTO users (name, email, age) VALUES (?, ?, ?)",
1696-
[123, "[email protected]", "thirty"]) # ← age is string, not INTEGER
1697-
# Error: "Type mismatch" (SQLite enforces STRICT)
1698-
```
1699-
17001608
#### ALTER COLUMN Support (libSQL Extension)
17011609

17021610
LibSQL supports modifying column attributes with ALTER COLUMN (not available in standard SQLite):

‎lib/ecto/adapters/libsql/connection.ex‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,19 @@ defmodule Ecto.Adapters.LibSql.Connection do
372372
end
373373

374374
defp column_options(opts, composite_pk) do
375+
# Validate generated column constraints (SQLite disallows these combinations).
376+
if Keyword.has_key?(opts, :generated) do
377+
if Keyword.has_key?(opts, :default) do
378+
raise ArgumentError,
379+
"generated columns cannot have a DEFAULT value (SQLite constraint)"
380+
end
381+
382+
if Keyword.get(opts, :primary_key) do
383+
raise ArgumentError,
384+
"generated columns cannot be part of a PRIMARY KEY (SQLite constraint)"
385+
end
386+
end
387+
375388
default = column_default(Keyword.get(opts, :default))
376389
null = if Keyword.get(opts, :null) == false, do: " NOT NULL", else: ""
377390

‎lib/ecto_libsql/native.ex‎

Lines changed: 94 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -284,15 +284,16 @@ defmodule EctoLibSql.Native do
284284
end
285285

286286
@doc false
287+
# Returns list on success, {:error, reason} on failure.
287288
def normalize_arguments(conn_id, statement, args) do
288-
# If args is already a list, return as-is (positional parameters)
289+
# If args is already a list, return as-is (positional parameters).
289290
case args do
290291
list when is_list(list) ->
291292
list
292293

293294
map when is_map(map) ->
294-
# Convert named parameters map to positional list
295-
# We need to introspect the statement to get parameter names and order them
295+
# Convert named parameters map to positional list.
296+
# Returns list on success, {:error, reason} on preparation failure.
296297
map_to_positional_args(conn_id, statement, map)
297298

298299
_ ->
@@ -310,57 +311,87 @@ defmodule EctoLibSql.Native do
310311
end
311312
end
312313

314+
# Cache key for parameter metadata.
315+
@param_cache_key {__MODULE__, :param_cache}
316+
317+
@doc false
318+
defp get_cached_param_names(statement) do
319+
case :persistent_term.get(@param_cache_key, nil) do
320+
nil -> nil
321+
cache -> Map.get(cache, statement)
322+
end
323+
end
324+
325+
@doc false
326+
defp cache_param_names(statement, param_names) do
327+
current = :persistent_term.get(@param_cache_key, %{})
328+
:persistent_term.put(@param_cache_key, Map.put(current, statement, param_names))
329+
param_names
330+
end
331+
313332
@doc false
314333
defp map_to_positional_args(conn_id, statement, param_map) do
315-
# Prepare the statement to introspect parameters
334+
# Check cache first to avoid repeated preparation overhead.
335+
case get_cached_param_names(statement) do
336+
nil ->
337+
# Cache miss - introspect and cache parameter names.
338+
# Returns list on success, {:error, reason} on failure.
339+
introspect_and_cache_params(conn_id, statement, param_map)
340+
341+
param_names ->
342+
# Cache hit - convert map to positional list using cached order.
343+
Enum.map(param_names, fn name ->
344+
Map.get(param_map, name, nil)
345+
end)
346+
end
347+
end
348+
349+
@doc false
350+
defp introspect_and_cache_params(conn_id, statement, param_map) do
351+
# Prepare the statement to introspect parameters.
316352
stmt_id = prepare_statement(conn_id, statement)
317353

318-
# stmt_id is a string UUID on success, or error tuple on failure
354+
# stmt_id is a string UUID on success, or error tuple on failure.
319355
case stmt_id do
320356
stmt_id when is_binary(stmt_id) ->
321-
# Get parameter count
357+
# Get parameter count.
322358
param_count =
323359
case statement_parameter_count(conn_id, stmt_id) do
324360
count when is_integer(count) -> count
325361
_ -> 0
326362
end
327363

328-
# Extract parameters in order
329-
args =
364+
# Extract parameter names in order.
365+
param_names =
330366
Enum.map(1..param_count, fn idx ->
331367
case statement_parameter_name(conn_id, stmt_id, idx) do
332368
name when is_binary(name) ->
333-
# Remove prefix (:, @, $) if present
334-
clean_name = remove_param_prefix(name) |> String.to_atom()
335-
336-
Map.get(param_map, clean_name, nil)
369+
# Remove prefix (:, @, $) if present.
370+
remove_param_prefix(name) |> String.to_atom()
337371

338372
nil ->
339-
# Positional parameter (?)
373+
# Positional parameter (?) - use nil as marker.
340374
nil
341375

342376
_ ->
343377
nil
344378
end
345379
end)
346380

347-
# Clean up prepared statement
381+
# Clean up prepared statement.
348382
close_stmt(stmt_id)
349383

350-
# Filter out any nils that might have come from positional params
351-
# If any parameter was not found in the map, we have an error
352-
# but we'll let the database handle it
353-
args
384+
# Cache the parameter names for future calls.
385+
cache_param_names(statement, param_names)
354386

355-
{:error, _reason} ->
356-
# If we can't prepare the statement, fall back to assuming it's positional
357-
# The actual execution will fail with a proper error
358-
if is_map(param_map) do
359-
# Convert map values to list in some order
360-
Map.values(param_map)
361-
else
362-
param_map
363-
end
387+
# Convert map to positional list using the names.
388+
Enum.map(param_names, fn name ->
389+
Map.get(param_map, name, nil)
390+
end)
391+
392+
{:error, reason} ->
393+
# Propagate the preparation error to callers.
394+
{:error, reason}
364395
end
365396
end
366397

@@ -375,9 +406,22 @@ defmodule EctoLibSql.Native do
375406
%EctoLibSql.Query{statement: statement} = query,
376407
args
377408
) do
378-
# Convert named parameters (map) to positional parameters (list)
379-
args_for_execution = normalize_arguments(conn_id, statement, args)
409+
# Convert named parameters (map) to positional parameters (list).
410+
# Returns {:error, reason} if parameter introspection fails.
411+
case normalize_arguments(conn_id, statement, args) do
412+
{:error, reason} ->
413+
{:error,
414+
%EctoLibSql.Error{
415+
message: "Failed to prepare statement for parameter introspection: #{reason}"
416+
}, state}
417+
418+
args_for_execution ->
419+
do_query(conn_id, mode, syncx, statement, args_for_execution, query, state)
420+
end
421+
end
380422

423+
@doc false
424+
defp do_query(conn_id, mode, syncx, statement, args_for_execution, query, state) do
381425
case query_args(conn_id, mode, syncx, statement, args_for_execution) do
382426
%{
383427
"columns" => columns,
@@ -427,21 +471,34 @@ defmodule EctoLibSql.Native do
427471
%EctoLibSql.Query{statement: statement} = query,
428472
args
429473
) do
430-
# Convert named parameters (map) to positional parameters (list)
431-
args_for_execution = normalize_arguments(conn_id, statement, args)
474+
# Convert named parameters (map) to positional parameters (list).
475+
# Returns {:error, reason} if parameter introspection fails.
476+
case normalize_arguments(conn_id, statement, args) do
477+
{:error, reason} ->
478+
{:error,
479+
%EctoLibSql.Error{
480+
message: "Failed to prepare statement for parameter introspection: #{reason}"
481+
}, state}
432482

433-
# Detect the command type to route correctly
483+
args_for_execution ->
484+
do_execute_with_trx(conn_id, trx_id, statement, args_for_execution, query, state)
485+
end
486+
end
487+
488+
@doc false
489+
defp do_execute_with_trx(conn_id, trx_id, statement, args_for_execution, query, state) do
490+
# Detect the command type to route correctly.
434491
command = detect_command(statement)
435492

436-
# For SELECT statements (even without RETURNING), use query_with_trx_args
437-
# For INSERT/UPDATE/DELETE with RETURNING, use query_with_trx_args
438-
# For INSERT/UPDATE/DELETE without RETURNING, use execute_with_transaction
439-
# Use word-boundary regex to detect RETURNING precisely (matching Rust NIF behavior)
493+
# For SELECT statements (even without RETURNING), use query_with_trx_args.
494+
# For INSERT/UPDATE/DELETE with RETURNING, use query_with_trx_args.
495+
# For INSERT/UPDATE/DELETE without RETURNING, use execute_with_transaction.
496+
# Use word-boundary regex to detect RETURNING precisely (matching Rust NIF behaviour).
440497
has_returning = Regex.match?(~r/\bRETURNING\b/i, statement)
441498
should_query = command == :select or has_returning
442499

443500
if should_query do
444-
# Use query_with_trx_args for SELECT or statements with RETURNING
501+
# Use query_with_trx_args for SELECT or statements with RETURNING.
445502
case query_with_trx_args(trx_id, conn_id, statement, args_for_execution) do
446503
%{
447504
"columns" => columns,

‎test/ecto_migration_test.exs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,5 +841,34 @@ defmodule Ecto.Adapters.LibSql.MigrationTest do
841841
assert sql =~ "STORED"
842842
assert sql =~ "price * quantity"
843843
end
844+
845+
test "rejects generated column with default value" do
846+
table = %Table{name: :users, prefix: nil}
847+
848+
columns = [
849+
{:add, :id, :id, [primary_key: true]},
850+
{:add, :computed, :string, [generated: "some_expr", default: "fallback"]}
851+
]
852+
853+
assert_raise ArgumentError,
854+
"generated columns cannot have a DEFAULT value (SQLite constraint)",
855+
fn ->
856+
Connection.execute_ddl({:create, table, columns})
857+
end
858+
end
859+
860+
test "rejects generated column as primary key" do
861+
table = %Table{name: :users, prefix: nil}
862+
863+
columns = [
864+
{:add, :computed_id, :integer, [primary_key: true, generated: "rowid * 1000"]}
865+
]
866+
867+
assert_raise ArgumentError,
868+
"generated columns cannot be part of a PRIMARY KEY (SQLite constraint)",
869+
fn ->
870+
Connection.execute_ddl({:create, table, columns})
871+
end
872+
end
844873
end
845874
end

‎test/error_handling_test.exs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,8 @@ defmodule EctoLibSql.ErrorHandlingTest do
221221
# Cleanup
222222
EctoLibSql.Native.close(real_conn_id, :conn_id)
223223
File.rm(test_db)
224+
File.rm(test_db <> "-wal")
225+
File.rm(test_db <> "-shm")
224226
end
225227
end
226228

0 commit comments

Comments
 (0)