ecto_ch icon indicating copy to clipboard operation
ecto_ch copied to clipboard

Add `with_cte_expression`

Open macobo opened this issue 10 months ago • 4 comments

ClickHouse supports two different sorts of CTEs:

  1. Normal, subquery based CTEs supported by most DBMS
  2. Expression-based CTEs

This PR adds support for (2) by adding a new adapter-specific method. Note the SQL syntax is flipped for (2).

Implementation is a hack - let me know if there are better solutions!

macobo avatar Feb 10 '25 08:02 macobo

👋 @macobo

If it's time-sensitive, I am happy to merge it but maybe as undocumented function/macro to avoid anyone outside of Plausible using it until we verify there is no other way to support this type of CTEs.

ruslandoga avatar Feb 10 '25 10:02 ruslandoga

Not in a rush right now - I thought this might simplify a task I'm working on, but I might be taking another approach.

macobo avatar Feb 10 '25 13:02 macobo

Small note for if you do end up reaching out to ecto developers: Keeping opts passed to with_cte in the same format, not validating for extra keys and making sure they get passed to connection.ex#cte would be ideal from an extensibility point of view. This would allow something like:

q
|> with_cte(fragment("123"), as: "foo", expression: true)

And the adapter can take care of the rest

macobo avatar Feb 10 '25 13:02 macobo

Definitely! But I don't think this alone would be enough for this example to work, since Ecto would try to ensure that "foo" is a valid CTE and fragment("123") is a valid CTE name and right now it fails at compile time before the adapter receives anything. with_cte("foo", as: fragment("123"), expression: true) would probably work however.

ruslandoga avatar Feb 11 '25 10:02 ruslandoga