Skip to content

Conversation

@greg-rychlewski
Copy link
Member

Right now we are passing {:constant, [], [value]} to the adapter and the adapter is taking care of the unpacking. But we can pass the constant value directly from Ecto and the existing adapter functionality can take care of it without the need for handling a :constant tuple.

@greg-rychlewski
Copy link
Member Author

greg-rychlewski commented Jan 9, 2026

The integration test failure is related to TDS and how the adapter was handling :constant in the past. It looks like it wasn't injecting strings correctly.

The failure:

* |   1) test fragments (Ecto.Adapters.TdsTest)
     +integration-test *failed* |      test/ecto/adapters/tds_test.exs:690
     +integration-test *failed* |      Assertion with == failed
     +integration-test *failed* |      code:  assert all(query) == ~s"SELECT 'let''s escape' FROM [schema] AS s0"
     +integration-test *failed* |      left:  "SELECT N'let''s escape' FROM [schema] AS s0"
     +integration-test *failed* |      right: "SELECT 'let''s escape' FROM [schema] AS s0"

The reason is because string literals are injected this way:

defp expr(string, _sources, _query) when is_binary(string) do
    "N'#{escape_string(string)}'"
end

But :constant was injected this way

defp expr({:constant, _, [literal]}, _sources, _query) when is_binary(literal) do
  [?', escape_string(literal), ?']
end

I can fix the test in the an ecto sql pr

@josevalim
Copy link
Member

Sounds good!

@greg-rychlewski greg-rychlewski merged commit 7224c12 into elixir-ecto:master Jan 9, 2026
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants