fuelen / ecto_dev_logger

An alternative logger for Ecto queries
Apache License 2.0
151 stars 16 forks source link

Support custom/non-standard types #16

Closed zachallaun closed 1 year ago

zachallaun commented 1 year ago

Hi there!

I just tried to give this a go and unfortunately ran into an immediate snag:

[error] module=telemetry - Handler [:ecto_dev_logger, :my_app, :repo] has failed and has been detached. Class=:error
Reason=:function_clause
Stacktrace=[
  {Ecto.DevLogger, :stringify_ecto_params,
   [%Postgrex.INET{address: {100, 100, 100, 100}, netmask: nil}, :root],
   [file: 'lib/ecto/dev_logger.ex', line: 233]},
  {Ecto.DevLogger, :"-inline_params/4-fun-1-", 3,
   [file: 'lib/ecto/dev_logger.ex', line: 109]},
  {Regex, :apply_list, 5, [file: 'lib/regex.ex', line: 756]},
  ....
]

I'm using a custom type that wraps %Postgrex.INET{}, which is the struct that Postgrex returns when using the inet underlying database type in Postgres.

It seems that stringify_ecto_params/2 doesn't have a fallback for dealing with structs that it doesn't already know about, hence the function clause error. A fallback that simply uses to_string would be neat, since someone could then implement String.Chars for their struct if they wished in order to format things.

fuelen commented 1 year ago

Hi @zachallaun

Falling back to String.Chars is not durable, as output format may differ from what is OK for DB. %Postgrex.INET{} is not a custom postgrex type, it is built-in, so we should have a solution for this inside the library (PR is welcome!). Users shouldn't define implementation for common structs. Here is a comment to the related topic: https://github.com/fuelen/ecto_dev_logger/pull/7#issuecomment-1107472681

Having a custom protocol is a plan for the future as it allows usage of custom postgrex types, but I don't want to implement it now, as personally I don't use such types, not sure if there are other users, and I'm not sure about good protocol interface for now.

fuelen commented 1 year ago

Hey @zachallaun

I've added support for INET and MACADDR types. Could you check main if it works for you? https://github.com/fuelen/ecto_dev_logger/commit/bef1be8b3cb2a91a4b8b04bd3c1ef466caed2d09

zachallaun commented 1 year ago

Thanks! Will give it a try tomorrow and get back to you.

zachallaun commented 1 year ago

Sorry for the delay; works great!

fuelen commented 1 year ago

Thanks. Published a new release with these changes.