[Proposal] make fragments look like string interpolation

54 views
Skip to first unread message

Daniel Kukuła

unread,
Mar 7, 2025, 6:29:28 AMMar 7
to elixir-ecto
I'm reading a fragment and it's very hard to read.
          fragment(
            "CASE WHEN ? IS NOT NULL THEN sum(?) filter (where ? = ANY (?)) ELSE sum(?) END",
            pps.id,
            ppi.price,
            ppi.status,
            ^total_charge_statuses(),
            ppi.price
          ),
But fragment is a macro -  we can generate the string with question marks from user input and then pass it as usual:
          fragment(
            "CASE WHEN #{pps.id} IS NOT NULL THEN sum(#{ppi.price}) filter (where #{ppi.status} = ANY (#{^total_charge_statuses()})) ELSE sum(#{ppi.price}) END",
),
This would be converted to the code as above and then to ast, or directly to ast.
To be clear, I'm not saying to interpolate it - it should just look like it. 

José Valim

unread,
Mar 7, 2025, 6:37:34 AMMar 7
to elixi...@googlegroups.com
I understand the issue you are talking about but we didn't make it look like a string interpolation on purpose because interpolating inside SQL, even in fragments, is not a habit we should instill into developers. Every time you see interpolation inside SQL, an alarm should ring in your head, it should not be contextual.


--
You received this message because you are subscribed to the Google Groups "elixir-ecto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/elixir-ecto/f75e797f-cbf8-47a7-979f-361388ff0400n%40googlegroups.com.

Daniel Kukuła

unread,
Mar 7, 2025, 6:51:02 AMMar 7
to elixir-ecto
we could use {{}} instead elixir string interpolations to distinguish that it's not a real string interpolation. 

Allen Madsen

unread,
Mar 7, 2025, 10:11:50 AMMar 7
to elixi...@googlegroups.com

benjamin...@gmail.com

unread,
Mar 7, 2025, 1:23:08 PMMar 7
to elixir-ecto
I believe you're looking for something like https://github.com/elixir-dbvisor/sql, changing the current behavior with Ecto.Query would only lead to confusion, and when doing raw SQL, you should know what you're doing. That said, I do think Ecto.SQL can do better then offer users a way to shoot them self in the foot by allowing String.t/iodata in query/2 and query!/2

danie...@gmail.com

unread,
Mar 7, 2025, 2:44:02 PMMar 7
to elixir-ecto
It's not about the fragment syntax, I just saw fragment that have 20 question marks mixed with some case statements.
Moving this to a macro will make it even more convoluted.
dbvisor seems has a bit different purpose than fragments.

I was thinking about something like this - I'm not even sure if the fragment is valid:
defmodule InterpolatedFragment do
  defmacro defrag(ast) do
    string = Macro.to_string(ast)
    pattern = ~r/\#\{([^\}]+)\}/

    vars =
      Regex.scan(pattern, string)
      |> Enum.map(fn [_, var] -> var end)
      |> Enum.join(", ")

    new_string = Regex.replace(pattern, string, "?")

    "fragment(#{new_string}, #{vars})"
    |> Code.string_to_quoted!
  end
end

defmodule YourModule do
  import InterpolatedFragment
  import Ecto.Query

  def your_query(star, table) do
    from q in defrag("select #{^star} from #{^table}")
  end
 
  def ecto_query(star, table) do
    from q in fragment("select ? from ?", ^star, ^table)
  end  
end

star = "column"
table = "users"
YourModule.your_query(star, table)

#Ecto.Query<from f0 in fragment("select ? from ?", ^"column", ^"users")>

benjamin...@gmail.com

unread,
Mar 7, 2025, 2:56:24 PMMar 7
to elixir-ecto
Yeah, I see what you’re saying and tend to agree, macro can become complicated. Not something I would advocate to pursue.

I want to clarify that what I linked to is not DBVisor, but the package sql, which gives you an sql sigil to safely construct parameterized queries.

Daniel Kukuła

unread,
Mar 8, 2025, 1:06:34 AMMar 8
to elixi...@googlegroups.com
I published it on hex https://hex.pm/packages/defrag

Daniel Kukuła
Software Engineer
Remote, UK
Fresha logo

Reply all
Reply to author
Forward
0 new messages