Add Ecto adapter by svan-jansson · Pull Request #3 · svan-jansson/ex_sql_client

svan-jansson · 2026-03-03T20:24:16Z

Pull request overview

This PR adds an Ecto 3 SQL adapter for ExSqlClient targeting Microsoft SQL Server, including SQL generation (MSSQL dialect) and result-shaping so Ecto can load rows positionally.

Changes:

Introduce ExSqlClient.Ecto adapter entrypoint and ExSqlClient.Ecto.Connection for SQL compilation + execution bridging to DBConnection.
Normalize DML results to expose affected-row counts (via a synthetic __rows_affected__ row injected by the .NET adapter).
Add unit tests for SQL generation and integration tests for end-to-end Ecto Repo operations; update docs and deps.

Copilot

Pull request overview

This PR adds an Ecto 3 SQL adapter for ExSqlClient targeting Microsoft SQL Server, including SQL generation (MSSQL dialect) and result-shaping so Ecto can load rows positionally.

Changes:

Introduce ExSqlClient.Ecto adapter entrypoint and ExSqlClient.Ecto.Connection for SQL compilation + execution bridging to DBConnection.
Normalize DML results to expose affected-row counts (via a synthetic __rows_affected__ row injected by the .NET adapter).
Add unit tests for SQL generation and integration tests for end-to-end Ecto Repo operations; update docs and deps.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/ecto/query_test.exs	Unit tests validating generated MSSQL SQL for common Ecto queries.
test/ecto/ecto_adapter_test.exs	Integration tests running Ecto Repo operations against SQL Server.
mix.exs	Adds `ecto`/`ecto_sql` deps (currently marked optional).
mix.lock	Locks `decimal`, `ecto`, and `ecto_sql` for the adapter.
lib/ex_sql_client/query.ex	Adds `String.Chars` impl for logging/string conversion of queries.
lib/ex_sql_client/ecto/connection.ex	Implements Ecto SQL connection callbacks, SQL generation, param encoding, and result normalization.
lib/ex_sql_client/ecto.ex	Adapter entry module (`use Ecto.Adapters.SQL`) + boolean type mapping.
dotnet/dotnet_sql_client/SqlAdapter.cs	Injects a synthetic affected-row count result for DML without OUTPUT.
README.md	Documents installation and usage for both direct driver usage and the new Ecto adapter.
AGENTS.md	Updates repository layout documentation to include the new Ecto components/tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-03T21:03:25Z

+Pass parameters as a map with string keys. Use `@paramName` placeholders in your SQL:
+
+```elixir
+{:ok, rows} =
+  ExSqlClient.query(conn, "SELECT * FROM [records] WHERE [status] = @status", %{status: 1})
+
+# rows is a list of maps, one map per row, with string column names as keys
+# e.g. [%{"id" => 1, "status" => 1, "name" => "foo"}, ...]


The README says "Pass parameters as a map with string keys", but the examples (and existing tests) use atom keys (e.g. %{status: 1}). If atom keys are supported (as the code/tests suggest), please update the text to avoid misleading users; otherwise update examples to use string keys consistently.

Copilot · 2026-03-03T21:03:26Z

+
+  # Synthetic row injected by the .NET side for DML without an OUTPUT clause.
+  # The C# layer sets this when ExecuteReader returns no rows but RecordsAffected >= 0.
+  defp normalize_result([%{"__rows_affected__" => n}], _col_order) do


normalize_result/2 treats any single-row result containing the key "rows_affected" as the synthetic DML marker, even if the query actually selected a real column with that name (map pattern matches maps with additional keys too). This can incorrectly drop real result rows. Consider tightening the match (e.g., require map_size(row) == 1) or using a less collision-prone marker structure.

Suggested change

defp normalize_result([%{"__rows_affected__" => n}], _col_order) do

defp normalize_result([%{"__rows_affected__" => n} = row], _col_order) when map_size(row) == 1 do

Copilot · 2026-03-03T21:03:26Z

+  # Split a SELECT projection list on commas that are NOT inside parentheses.
+  defp split_select_list(str) do
+    {items, current, _depth} =
+      String.graphemes(str)
+      |> Enum.reduce({[], "", 0}, fn
+        "(", {items, current, depth} -> {items, current <> "(", depth + 1}
+        ")", {items, current, depth} -> {items, current <> ")", depth - 1}
+        ",", {items, current, 0} -> {[current | items], "", 0}
+        char, {items, current, depth} -> {items, current <> char, depth}
+      end)
+
+    Enum.reverse([current | items])
+  end
+


split_select_list/1 splits on commas outside parentheses, but it doesn’t account for commas inside string literals (e.g. SELECT N'a,b' AS [x] ...). That can make column_order_from_sql/1 return the wrong column order and cause Ecto to load fields positionally into the wrong columns. Consider enhancing the splitter to track quote state (single-quoted strings) so commas inside strings don’t split items.

Suggested change

# Split a SELECT projection list on commas that are NOT inside parentheses.

defp split_select_list(str) do

{items, current, _depth} =

String.graphemes(str)

|> Enum.reduce({[], "", 0}, fn

"(", {items, current, depth} -> {items, current <> "(", depth + 1}

")", {items, current, depth} -> {items, current <> ")", depth - 1}

",", {items, current, 0} -> {[current | items], "", 0}

char, {items, current, depth} -> {items, current <> char, depth}

end)

Enum.reverse([current | items])

end

# Split a SELECT projection list on commas that are NOT inside parentheses

# and NOT inside single-quoted string literals.

defp split_select_list(str) when is_binary(str) do

do_split_select_list(str, [], "", 0, false)

end

# Recursive worker:

# - items: accumulated list of completed items (in reverse order)

# - current: current item being built

# - depth: parenthesis nesting depth

# - in_string: whether we are inside a single-quoted string literal

defp do_split_select_list(<<>>, items, current, _depth, _in_string) do

Enum.reverse([current | items])

end

# Handle escaped single quote inside a string: '' → a literal '

defp do_split_select_list(<<"''", rest::binary>>, items, current, depth, true) do

do_split_select_list(rest, items, current <> "''", depth, true)

end

# Closing single quote for a string literal.

defp do_split_select_list(<<"'", rest::binary>>, items, current, depth, true) do

do_split_select_list(rest, items, current <> "'", depth, false)

end

# Opening single quote for a string literal.

defp do_split_select_list(<<"'", rest::binary>>, items, current, depth, false) do

do_split_select_list(rest, items, current <> "'", depth, true)

end

# Opening parenthesis outside a string: increase depth.

defp do_split_select_list(<<"(", rest::binary>>, items, current, depth, false) do

do_split_select_list(rest, items, current <> "(", depth + 1, false)

end

# Closing parenthesis outside a string: decrease depth (but not below 0).

defp do_split_select_list(<<")", rest::binary>>, items, current, depth, false) do

new_depth = if depth > 0, do: depth - 1, else: 0

do_split_select_list(rest, items, current <> ")", new_depth, false)

end

# Comma at top level and outside a string: split here.

defp do_split_select_list(<<",", rest::binary>>, items, current, 0, false) do

do_split_select_list(rest, [current | items], "", 0, false)

end

# Any other character (including commas inside strings or inside parentheses).

defp do_split_select_list(<<char::utf8, rest::binary>>, items, current, depth, in_string) do

do_split_select_list(rest, items, current <> <<char::utf8>>, depth, in_string)

end

Copilot · 2026-03-03T21:03:26Z

+  defp expr({:in, _, [left, right]}, sources, query) do
+    [expr(left, sources, query), " = ANY(", expr(right, sources, query) | ")"]


The fallback expr/3 for :in generates = ANY(...), which is not valid SQL Server syntax. For MSSQL, this should likely be an error (unsupported expression) or be compiled to an IN (...) form that SQL Server supports, depending on what Ecto can emit here.

Suggested change

defp expr({:in, _, [left, right]}, sources, query) do

[expr(left, sources, query), " = ANY(", expr(right, sources, query) | ")"]

defp expr({:in, _, [_left, _right]}, _sources, query) do

error!(query, "ExSqlClient.Ecto does not support IN with non-list or subquery right-hand side")

Copilot · 2026-03-03T21:03:27Z

@@ -0,0 +1,1197 @@
+if Code.ensure_loaded?(Ecto.Adapters.SQL.Connection) do
+defmodule ExSqlClient.Ecto.Connection do


These guard-wrapped modules appear unformatted: defmodule is not indented inside the if Code.ensure_loaded?/1 block. This will likely fail mix format --check-formatted. Run mix format (or indent the module body within the if block).

Suggested change

defmodule ExSqlClient.Ecto.Connection do

defmodule ExSqlClient.Ecto.Connection do

Copilot · 2026-03-03T21:03:27Z

+if Code.ensure_loaded?(Ecto.Adapters.SQL) do
+defmodule ExSqlClient.Ecto do


This module is not indented inside the if Code.ensure_loaded?/1 block, which will likely fail mix format --check-formatted. Please run mix format (or indent the defmodule body under the if).

Copilot · 2026-03-03T21:03:27Z

+There are two categories of tests:
+
+**Unit tests** (no database required) — SQL generation tests for the Ecto adapter:
+
+```bash
+mix test test/ecto/query_test.exs
+```
+
+**Integration tests** spin up a SQL Server container automatically via
+[Testcontainers](https://hex.pm/packages/testcontainers). Docker or a
+compatible rootless runtime (e.g. Podman) must be available.


This section says the Ecto adapter SQL-generation tests require no database, but test/test_helper.exs currently starts Testcontainers and a SQL Server container unconditionally (even when running only test/ecto/query_test.exs). Either update test/test_helper.exs to only start containers when integration tests are enabled, or adjust this doc to reflect the current behavior.

svan-jansson added 2 commits March 3, 2026 21:19

Add Ecto support

b904c8d

Update docs

1c11fa0

svan-jansson requested a review from Copilot March 3, 2026 20:24

Update AGENTS.md

6e73a5a

Copilot started reviewing on behalf of svan-jansson March 3, 2026 20:24 View session

svan-jansson added 2 commits March 3, 2026 21:25

Update AGENTS.md

04fd812

Update AGENTS.md

6b1a0f7

Copilot AI reviewed Mar 3, 2026

View reviewed changes

svan-jansson added 3 commits March 3, 2026 21:34

Update README.md

5659c40

Fix credo warnings

4831da0

Review fixes

b80e2bc

svan-jansson requested a review from Copilot March 3, 2026 20:55

Copilot started reviewing on behalf of svan-jansson March 3, 2026 20:55 View session

svan-jansson merged commit de3a1a6 into master Mar 3, 2026
8 checks passed

svan-jansson deleted the feature/ecto-adapter branch March 3, 2026 20:59

Copilot AI reviewed Mar 3, 2026

View reviewed changes

	defp normalize_result([%{"__rows_affected__" => n}], _col_order) do
	defp normalize_result([%{"__rows_affected__" => n} = row], _col_order) when map_size(row) == 1 do

-  # Split a SELECT projection list on commas that are NOT inside parentheses.
-  defp split_select_list(str) do
-    {items, current, _depth} =
-      String.graphemes(str)
-      |> Enum.reduce({[], "", 0}, fn
-        "(", {items, current, depth} -> {items, current <> "(", depth + 1}
-        ")", {items, current, depth} -> {items, current <> ")", depth - 1}
-        ",", {items, current, 0} -> {[current | items], "", 0}
-        char, {items, current, depth} -> {items, current <> char, depth}
-      end)
-    Enum.reverse([current | items])
-  end
+  # Split a SELECT projection list on commas that are NOT inside parentheses
+  # and NOT inside single-quoted string literals.
+  defp split_select_list(str) when is_binary(str) do
+    do_split_select_list(str, [], "", 0, false)
+  end
+  # Recursive worker:
+  #   - items: accumulated list of completed items (in reverse order)
+  #   - current: current item being built
+  #   - depth: parenthesis nesting depth
+  #   - in_string: whether we are inside a single-quoted string literal
+  defp do_split_select_list(<<>>, items, current, _depth, _in_string) do
+    Enum.reverse([current | items])
+  end
+  # Handle escaped single quote inside a string: '' → a literal '
+  defp do_split_select_list(<<"''", rest::binary>>, items, current, depth, true) do
+    do_split_select_list(rest, items, current <> "''", depth, true)
+  end
+  # Closing single quote for a string literal.
+  defp do_split_select_list(<<"'", rest::binary>>, items, current, depth, true) do
+    do_split_select_list(rest, items, current <> "'", depth, false)
+  end
+  # Opening single quote for a string literal.
+  defp do_split_select_list(<<"'", rest::binary>>, items, current, depth, false) do
+    do_split_select_list(rest, items, current <> "'", depth, true)
+  end
+  # Opening parenthesis outside a string: increase depth.
+  defp do_split_select_list(<<"(", rest::binary>>, items, current, depth, false) do
+    do_split_select_list(rest, items, current <> "(", depth + 1, false)
+  end
+  # Closing parenthesis outside a string: decrease depth (but not below 0).
+  defp do_split_select_list(<<")", rest::binary>>, items, current, depth, false) do
+    new_depth = if depth > 0, do: depth - 1, else: 0
+    do_split_select_list(rest, items, current <> ")", new_depth, false)
+  end
+  # Comma at top level and outside a string: split here.
+  defp do_split_select_list(<<",", rest::binary>>, items, current, 0, false) do
+    do_split_select_list(rest, [current | items], "", 0, false)
+  end
+  # Any other character (including commas inside strings or inside parentheses).
+  defp do_split_select_list(<<char::utf8, rest::binary>>, items, current, depth, in_string) do
+    do_split_select_list(rest, items, current <> <<char::utf8>>, depth, in_string)
+  end

		defp expr({:in, _, [left, right]}, sources, query) do
		[expr(left, sources, query), " = ANY(", expr(right, sources, query) \| ")"]

		@@ -0,0 +1,1197 @@
		if Code.ensure_loaded?(Ecto.Adapters.SQL.Connection) do
		defmodule ExSqlClient.Ecto.Connection do

		if Code.ensure_loaded?(Ecto.Adapters.SQL) do
		defmodule ExSqlClient.Ecto do

Conversation

svan-jansson commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

svan-jansson commented Mar 3, 2026 •

edited

Loading