Thank you — and reusable SQL formatting

[halostatue]

I’ve implemented something like the formatter bits of ecto_dev_logger in the past, but your approach here was very clean and helped me with an investigation that I was looking for.

I have suggested to the Ecto team that Ecto (SQL) probably should have formatter functions and callbacks to the adapters for the differences as you have implemented, and that your formatter code might be a really good start for this.

If that's rejected, can I suggest extracting the SQL formatter code from ecto_dev_logger into a separate repository/package so that it is usable by others? We’ve written an inspect_sql function that has similar formatting functionality (focused only on PostgreSQL for now), but I think that if the Ecto team doesn’t have any interest in a formatter by default, having a community package focused on formatting would be the next best option.

[fuelen]

You're welcome :)

If that's rejected, can I suggest extracting the SQL formatter code from ecto_dev_logger into a separate repository/package so that it is usable by others?

Well, the current implementation is far from being perfect. It derives database-level types from Elixir types as Ecto telemetry events do not provide params types. ecto_dev_logger provides correct formatting only in a limited number of cases. There is an assumption that if the type is a map, then this is a JSON just because Ecto uses it. However, according to Data representation table of the Postgrex library, map is used for hstore in the first place. String, integer, float, array - all these types are convertible to JSON as well. There are other cases.
I think that a generic and universal library should be more strict and accept precise type info.

If you need inspect_sql function for internal stuff and not for a library, then you can simply use ecto_dev_logger as is

[halostatue]

I’m looking at SQL formatting from a library perspective because of the need to capture migration SQL. Where extracting an SQL formatter might be useful (IMO) is that it would hopefully attract other contributors who would be able to extend it to behave correctly in more cases. As of right now, my implementation's correctness is probably even less than yours:

  defp format_sql_parameter({{y, m, d}, {_h, _m, _s, _ms}}) do
    "'#{y}-#{m}-#{d}'"
  end

  defp format_sql_parameter(value) when is_binary(value) do
    value =
      case Ecto.UUID.cast(value) do
        {:ok, uuid} -> uuid
        _ -> value
      end

    "'#{value}'"
  end

  defp format_sql_parameter(value) when is_list(value) do
    "ARRAY[#{Enum.map_join(value, ", ", &format_sql_parameter/1)}]"
  end

  defp format_sql_parameter(%NaiveDateTime{} = value) do
    "'#{NaiveDateTime.to_iso8601(value)}'"
  end

  defp format_sql_parameter(%DateTime{} = value) do
    "'#{DateTime.to_iso8601(value)}'"
  end

  defp format_sql_parameter(value) when is_number(value) do
    to_string(value)
  end

  defp format_sql_parameter(value) do
    "'#{value}'"
  end

[fuelen]

Having a dedicated library for formatting parameters is a good idea. But for the time being, I'd suggest just using ecto_dev_logger as a dependency. Besides the formatting logic, it has a couple of functions for handling telemetry events, and that's it. Not a lot of garbage :)

[halostatue]

The biggest "issue" with using the provided formatter by default is that for some cases, I wouldn’t want the ANSI escape codes (again, see my initial use case).

Like I said, my main thought here was that this might be a useful direction for the future. It’s not an immediate need now.

[halostatue]

I think that my needs for SQL formatting could be achieved if there were a way to call EctoDevLogger's SQL formatter code to suppress colouring.

[fuelen]

Ecto.DevLogger.inline_params/4 function does what you need. However, it is undocumented for now. Colouring is applied only if IO.ANSI.enabled?, so you can disable it with Application.put_env(:elixir, :ansi_enabled, true).
PR for making the function public and colouring explicit is welcome :)

[fuelen]

I think it is better to make inline_params public after solving this issue: #9 (comment)