Proposal: expose param_types to telemetry metadata

[Artur Plysiuk]

I'd like to suggest adding param_types to telemetry metadata in order to finally resolve a couple of issues in ecto_dev_logger. param_types should contain a list of adapter-specific types that were really used for making a query. Example: [Postgrex.Extensions.UUID, Postgrex.Extensions.JSONB].

In the current implementation, the library guesses DB types by looking at values represented by params key. It works for many cases, but not for all. If a column has json(b) type, it is possible to store many different values there, like lists, numbers, strings, booleans. At the moment the library can't properly handle these cases and only maps are represented as json in the end. A json with list of maps will be treated as {:array, :map) and this may be wrong. There is a logic, where if something looks like UUID, then most-likely it is a UUID. And "America/New_York" looks like a valid UUID, thus converted to loaded value:

> Ecto.UUID.load("America/New_York")
{:ok, "416d6572-6963-612f-4e65-775f596f726b"}

[José Valim]

Is the recently added cast_params not enough?

--
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 on the web visit https://groups.google.com/d/msgid/elixir-ecto/3ea58c96-d6a3-4778-8e19-c76ee271c16an%40googlegroups.com.

[Artur Plysiuk]

Unfortunately, no.

Here is one of the cases, where people save structs to json field. In this case both cast_params and params are equal: [%PostData{foo: true}]. This representation is too abstract.

Another use case: %{"foo" => "bar"} can be represented both, as json and as hstore. Current metadata doesn't resolve these issues.

середа, 11 жовтня 2023 р. о 13:39:21 UTC+3 José Valim пише:

[José Valim]

Sorry, it is not clear to me. params gives you all params as to-be-written to the database. cast_params give the original values. Zipping those should give you before and after. What am I missing? :)

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/f92e9588-0cb2-49e1-8d1a-f6f37fc7d7f0n%40googlegroups.com.

[Austin Ziegler]

I think that what’s being requested is the nominal type—the transformer that gets called. Having the before and after can correlate, but not know what the target schema types.

-a

On Oct 11, 2023, at 06:56, José Valim <jose....@dashbit.co> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAGnRm4%2B%3D_WazxGDcozSZzbud4JtovQNwXfV8Z%3D2ANUgAUeyGnQ%40mail.gmail.com.

[Artur Plysyuk]

I want to inline parameters into the query. I want to represent them as strings.

if I have %{"foo" => "bar"}, I can represent it in

JSON syntax: '{"foo":"bar"}'

HSTORE syntax: '"foo"=>"bar"'

It is not clear which one should I use. param_types should help to detect which type was really used by the database.

Here is an example of metadata when a struct is written to json field:

  cast_params: [%PostData{foo: true}],
  params: [%PostData{foo: true}],
  query: "INSERT INTO \"posts\" (\"data\") VALUES ($1) RETURNING \"id\"",

zipping won't help :)

You received this message because you are subscribed to a topic in the Google Groups "elixir-ecto" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-ecto/5AwUoaEuq4E/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-ecto...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/E55A1090-C2CC-4284-93EA-FBC60800EA4E%40gmail.com.

[José Valim]

> I want to inline parameters into the query. I want to represent them as strings.

That answers it, thanks! We could add a cast_types field, I believe. Or ideally we add a new field with the cast value, dumped value, and type, deprecating the cast_params one. If you want to submit a PR, that would be welcome.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAPhto9kAOX%3Dmg8tDNX%3DKMbyyXqycNcSGLHtitS8fqiP94YZvWQ%40mail.gmail.com.

[Artur Plysiuk]

I think cast_types name may be not a good fit here, as it is ambiguous. Based on name, I'd expect a list of ecto types instead of types of the lower level.

I was suggesting param_types name just because it is used by Postgrex.Query.

Once again, the desired data in metadata is these type identifiers [Postgrex.Extensions.UUID, Postgrex.Extensions.JSONB], not primitive ecto types.

My view is biased, as it is focused on postgres and I don't know whether such info about types is available if myxql/tds is used.

I see in docs, that param_types is not in the list of public fields. Actually, Postgrex.Extensions.* modules are not documented as well.

Seems like this is not that easy as I was thinking :/

середа, 11 жовтня 2023 р. о 16:38:13 UTC+3 José Valim пише:

[José Valim]

Oh, I see. Ecto actually doesn't know anything about the Postgrex types, it is completely transparent to us. :(

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/5ee0e4bf-ded8-41a2-b989-37f8d4476bcbn%40googlegroups.com.

[benwil...@gmail.com]

> I want to inline parameters into the query.

Just a note here that for logging purposes it's IMHO important to distinguish between which values are actually in the query string and which are parameters, as this impacts query planning. Inlining them in the logging layer would hide that and that doesn't seem desirable to me.

[Artur Plysiuk]

That's true. However, in ecto_dev_logger, parameters are visually distinguishable in the query, as they have different colors.

субота, 14 жовтня 2023 р. о 16:23:45 UTC+3 benwil...@gmail.com пише: