voger

How to add external variable to fragment?

I have read many topics and pages about this issue and while it seems this should work I am probably missing something.

I have this query

      from(v in Visit,
        right_join:
          day in fragment(
            "generate_series(CURRENT_DATE - INTERVAL '? day', CURRENT_DATE, '1 day')",
            30
          ),
        on: day == fragment("date(?)", v.inserted_at),
        group_by: day,
        select: %{
          day: fragment("date(?)", day),
          count: count(v.id)
        }
      )
      |> Repo.all()

Which returns

[
  %{count: 0, day: ~D[2019-09-03]},
  %{count: 0, day: ~D[2019-09-26]},
  %{count: 3, day: ~D[2019-09-15]},
  %{count: 0, day: ~D[2019-09-11]},
     ...
]

I want that 30 to be a variable so I change the query to

      num_of_days = 30

      from(v in Visit,
        right_join:
          day in fragment(
            "generate_series(CURRENT_DATE - INTERVAL '? day', CURRENT_DATE, '1 day')",
            ^num_of_days
          ),
        on: day == fragment("date(?)", v.inserted_at),
        group_by: day,
        select: %{
          day: fragment("date(?)", day),
          count: count(v.id)
        }
      )
      |> Repo.all()

This fails with

[debug] QUERY ERROR source="visits" db=9.5ms queue=1.4ms
SELECT date(f1), count(v0."id") FROM "visits" AS v0 RIGHT OUTER JOIN generate_series(CURRENT_DATE - INTERVAL '$1 day', CURRENT_DATE, '1 day') AS f1 ON f1 = date(v0."inserted_at") GROUP BY f1 [30]
** (ArgumentError) parameters must be of length 0 for query %Postgrex.Query{cache: :reference, columns: ["date", "count"], name: "ecto_1187", param_formats: [], param_oids: [], param_types: [], ref: #Reference<0.3361914078.3108241410.64835>, result_formats: [:binary, :binary], result_oids: [1082, 20], result_types: [Postgrex.Extensions.Date, Postgrex.Extensions.Int8], statement: "SELECT date(f1), count(v0.\"id\") FROM \"visits\" AS v0 RIGHT OUTER JOIN generate_series(CURRENT_DATE - INTERVAL '$1 day', CURRENT_DATE, '1 day') AS f1 ON f1 = date(v0.\"inserted_at\") GROUP BY f1", types: {Postgrex.DefaultTypes, #Reference<0.3361914078.3108372482.49372>}}
    (postgrex) lib/postgrex/query.ex:80: DBConnection.Query.Postgrex.Query.encode/3
    (db_connection) lib/db_connection.ex:1148: DBConnection.encode/5
    (db_connection) lib/db_connection.ex:1246: DBConnection.run_prepare_execute/5
    (db_connection) lib/db_connection.ex:1342: DBConnection.run/6
    (db_connection) lib/db_connection.ex:540: DBConnection.parsed_prepare_execute/5
    (db_connection) lib/db_connection.ex:533: DBConnection.prepare_execute/4
    (ecto_sql) lib/ecto/adapters/sql.ex:562: Ecto.Adapters.SQL.execute!/4
    (ecto_sql) lib/ecto/adapters/sql.ex:554: Ecto.Adapters.SQL.execute/5
    (ecto) lib/ecto/repo/queryable.ex:153: Ecto.Repo.Queryable.execute/4
    (ecto) lib/ecto/repo/queryable.ex:18: Ecto.Repo.Queryable.all/3

So the variable is not interpolated

I also tried the macro approach

  def count_by_date_with(identifier, num_of_days) do

    num_of_days = 30

    from(v in Visit,
      right_join: day in days(num_of_days),
      on: day == fragment("date(?)", v.inserted_at),
      group_by: day,
      select: %{
        day: fragment("date(?)", day),
        count: count(v.id)
      }
    )
    |> Repo.all()
  end

  defmacro days(num_of_days) do
    quote do
      fragment(
        "generate_series(CURRENT_DATE - INTERVAL '? day', CURRENT_DATE, '1 day')",
        unquote(num_of_days)
      )
    end
  end

But this doesn’t even compile

iex(25)> recompile
Compiling 1 file (.ex)

== Compilation error in file lib/tinyclone/visit.ex ==
** (Ecto.Query.CompileError) malformed join `days(num_of_days)` in query expression
    (ecto) lib/ecto/query.ex:755: Ecto.Query.from/5
    (ecto) expanding macro: Ecto.Query.from/2
    (tinyclone) lib/tinyclone/visit.ex:43: TinyClone.Visit.count_by_date_with/2
    (elixir) expanding macro: Kernel.|>/2
    (tinyclone) lib/tinyclone/visit.ex:52: TinyClone.Visit.count_by_date_with/2
    (elixir) lib/kernel/parallel_compiler.ex:229: anonymous fn/4 in Kernel.ParallelCompiler.spawn_workers/7
** (exit) shutdown: 1
    (mix) lib/mix/tasks/compile.all.ex:59: Mix.Tasks.Compile.All.do_compile/4
    (mix) lib/mix/tasks/compile.all.ex:24: anonymous fn/1 in Mix.Tasks.Compile.All.run/1
    (mix) lib/mix/tasks/compile.all.ex:40: Mix.Tasks.Compile.All.with_logger_app/1
    (mix) lib/mix/task.ex:331: Mix.Task.run_task/3
    (mix) lib/mix/tasks/compile.ex:96: Mix.Tasks.Compile.run/1
    (mix) lib/mix/task.ex:331: Mix.Task.run_task/3
    (iex) lib/iex/helpers.ex:104: IEx.Helpers.recompile/1

How can I interpolate that variable in the fragment?

2 comments

/phoenix #ecto #query #ecto-query #fragment

12 2393 2

2019-09-26 19:58:06 UTC

Marked As Solved

voger

Thank you. I was thinking in terms of string interpolation while Ecto sends prepared statements in the database. So I did a bit more digging and I concluded to this code

      from(v in Visit,
        right_join:
          day in fragment(
            "generate_series(CURRENT_DATE - CAST(? AS INTERVAL), CURRENT_DATE, '1 day')",
            ^%Postgrex.Interval{days: num_of_days}
          ),
        on: day == fragment("date(?)", v.inserted_at),
        group_by: day,
        select: %{
          day: fragment("date(?)", day),
          count: count(v.id)
        }
      )
      |> Repo.all()

and the result

[debug] QUERY OK source="visits" db=10.6ms queue=0.2ms
SELECT date(f1), count(v0."id") FROM "visits" AS v0 RIGHT OUTER JOIN generate_series(CURRENT_DATE - CAST($1 AS INTERVAL), CURRENT_DATE, '1 day') AS f1 ON f1 = date(v0."inserted_at") GROUP BY f1 [%Postgrex.Interval{days: 12, months: 0, secs: 0}]
[
  %{count: 0, day: ~D[2019-09-26]},
  %{count: 3, day: ~D[2019-09-15]},
  %{count: 0, day: ~D[2019-09-18]},
  %{count: 0, day: ~D[2019-09-19]},
  %{count: 0, day: ~D[2019-09-14]},
  %{count: 0, day: ~D[2019-09-20]},
  %{count: 0, day: ~D[2019-09-16]},
  %{count: 0, day: ~D[2019-09-24]},
  %{count: 0, day: ~D[2019-09-25]},
  %{count: 0, day: ~D[2019-09-23]},
  %{count: 0, day: ~D[2019-09-22]},
  %{count: 0, day: ~D[2019-09-17]},
  %{count: 0, day: ~D[2019-09-21]}
]

Thank you so much for pointing this out.

Post #3

Also Liked

LostKobrakai

That’s exactly what fragment is supposed to prevent. Interpolating data into the query itself opens you up to sql injection attacks by whomever is allowed to set the variable you’re using. Ecto doesn’t allow that and only uses parameters sent separately to the database.

What I’m wondering though is if you couldn’t “interpolate” the complete interval value outside of the query and only use the fragment for the whole string.

interval = "#{num_of_days} day"

… fragment(
            "generate_series(CURRENT_DATE - INTERVAL ?, CURRENT_DATE, '1 day')",
            ^interval
          )

Post #2