How would you refactor a code with "fragment" ecto macros?

Hi,

How would you refactor a code like this:

case filter do
  :by_day ->
    from(d in Download,
      where: d.release_id == ^release_id,
      group_by: fragment("date_trunc('day', ?)", d.day),
      order_by: fragment("date_trunc('day', ?)", d.day),
      select: {fragment("to_char(date_trunc('day', ?), 'YYYY-MM-DD')", d.day), fragment("sum(?)", d.downloads)})
  :by_week ->
    from(d in Download,
      where: d.release_id == ^release_id,
      group_by: fragment("date_trunc('week', ?)",  d.day),
      order_by: fragment("date_trunc('week', ?)",  d.day),
      select: {fragment("to_char(date_trunc('week', ?), 'YYYY-MM-DD')", d.day), fragment("sum(?)", d.downloads)})
  :by_month ->
    from(d in Download,
      where: d.release_id == ^release_id,
      group_by: fragment("date_trunc('month', ?)",  d.day),
      order_by: fragment("date_trunc('month', ?)",  d.day),
      select: {fragment("to_char(date_trunc('month', ?), 'YYYY-MM')", d.day), fragment("sum(?)", d.downloads)})
end

I was not able to do it because of the nature of date_trunc and fragment. I would like to just pass “day”, “month” or “week” and build dynamically the query.

Which refactors would you do?

case Atom.to_string(filter) do
  "by_" <> query_type ->
    from(d in Download,
      where: d.release_id == ^release_id,
      group_by: fragment("date_trunc(?, ?)", query_type, d.day),
      order_by: fragment("date_trunc(?, ?)", query_type , d.day),
      select: {fragment("to_char(date_trunc(?, ?), 'YYYY-MM-DD')", query_type, d.day), fragment("sum(?)", d.downloads)})
 _ -> # whatever?
end

wouldn’t this work?

1 Like

Your fragments can be macros:

defmacro trunc_by_day(expr) do
  quote do
    fragment("date_trunc('day', ?)", unquote(expr))
  end
end

And then you can import it and call trunc_by_day(d.day).

There is a chance this will work too:

defmacro date_trunc(period, expr) do
  quote do
    fragment("date_trunc(?, ?)", unquote(period), unquote(expr))
  end
end

but that will depend on the database syntax more than anything else.

Finally, sum is already in the Ecto.Query.API, so you don’t need to fragment it.

1 Like

I couldn’t achieve to do it. I tried but it fails.

  1) test GET /api/packages/:name/releases/:version/downloads get release downloads by day (Hexpm.Web.API.ReleaseControllerTest)
     test/hexpm/web/controllers/api/release_controller_test.exs:580
     ** (Postgrex.Error) ERROR 42803 (grouping_error): column "d0.day" must appear in the GROUP BY clause or be used in an aggregate function

It is something related with fragment (macros) and the SQL queries.

    filter = Atom.to_string(filter)
    case filter do
      "by_" <> period ->
        from(d in Download,
          where: d.release_id == ^release_id)
          group_by: date_trunc(^period, d.day),
          order_by: date_trunc(^period, d.day),
          select: {date_trunc_format(^period, "YYYY-MM-DD", d.day), sum(d.downloads)})
      ...
  end

Still, thanks :wink:

Thank you :wink:

Cool, the first part worked. The second, not.

This is the maximum refactor I achieved:

  defmacro date_trunc(period, expr) do
    quote do
      fragment("date_trunc(?, ?)", unquote(period), unquote(expr))
    end
  end
  defmacro date_trunc_format(period, format, expr) do
    quote do
      fragment("to_char(date_trunc(?, ?), ?)", unquote(period), unquote(expr), unquote(format))
    end
  end


  def func(release_id, filter) do
    query = from(d in Download,
      where: d.release_id == ^release_id)
    # IO.puts(filter)
    # filter = Atom.to_string(filter)
    case filter do
      # "by_" <> period ->
      #   from(d in Download,
      #     where: d.release_id == ^release_id)
      #     group_by: date_trunc(^period, d.day),
      #     order_by: date_trunc(^period, d.day),
      #     select: {date_trunc_format(^period, "YYYY-MM-DD", d.day), sum(d.downloads)})
      :by_day ->
        from(d in query,
          group_by: date_trunc("day", d.day),
          order_by: date_trunc("day", d.day),
          select: {date_trunc_format("day", "YYYY-MM-DD", d.day), sum(d.downloads)})
      :by_week ->
        from(d in query,
          group_by: date_trunc("week", d.day),
          order_by: date_trunc("week", d.day),
          select: {date_trunc_format("week", "YYYY-MM-DD", d.day), sum(d.downloads)})
      :by_month ->
        from(d in query,
          group_by: date_trunc("month", d.day),
          order_by: date_trunc("month", d.day),
          select: {date_trunc_format("month", "YYYY-MM", d.day), sum(d.downloads)})
      ...
   end
end

The commented part does not work, although It would be a great refactor (saving lines), the ^period does not work with fragment/SQL query.

I tried to reuse the value of group_by and order_by but I can not do it because it needs the d.day.

Do you think this is the maximum I can achieve? It is a bit of improvement but no so hard as I would expect.

Thanks again