[Proposal] Add callback to modify query before sending to database

[danie...@gmail.com]

I'm looking for a way to prepend/append comments to generated queries.
Why I think it's useful:
- Prepending a comment with name to a query: Datadog by default is using the actual query as the query name - this makes it very hard to distinguish where the query comes from in the database view because often the queries start with the same select statement. This is especially problematic when using micro services with a shared database because you need sometimes to search multiple services to find where a query was produced.
- Appending additional comment with data to query with sqlcommenter. It's part of opentelementry and there are libraries to use it with most programming languages: Go, Python, Php, Ruby, Node.js, Java. It would be nice to add elixir to the list.

I made a library that escapes the provided options according to the specs from the website so these are sql injection safe, but I could not find an easy way to modify the query.
I think the simplest way for it would be to modify postgrex to accept a function as a callback that will modify the query. It can accept io data.
Alternatively this could be implemented inside ecto.

[José Valim]

We can support something like that but it is important to note that Ecto prepares queries, so the values we support would be static in nature. So you can give the names but you can't attach runtime information.

--
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/6bb179bb-7e56-4602-9882-69af0de1476fn%40googlegroups.com.

[Yevhenii Kurtov]

The crucial bit is to stick to the principle of the least surprise and to not allow to modify the query in a way that it would return different results.
Adding comments in that respect is closer to enriching query for observability. 

Otherwise this feature will be hated with the passion of thousands hells. 

Best, 

Yevhenii 

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAGnRm4LvBk1MMX_hNTo%3DOavxLqpn1mAYkh-8n7p355w8zokEzg%40mail.gmail.com.

[danie...@gmail.com]

Adding static data could be a nice first step.
I have read the source and the runtime information would need to be attached after the prepared query is returned from ets. I will check how this is implemented in other languages/frameworks. I imagine some of them are also caching the generated query.

[danie...@gmail.com]

I did some more research:
There is a postgres extension - pg_tracing in the making that will use sqlcommenter. It's still being reviewed but it looks like it's close to release.
I checked the source code of a few libraries and they provide a predefined set of keys (django as example, laravel):
controller, route, framework, db_driver, app_name (only django), action (only laravel) - these could be all static.
and optionally the w3c trace-context which includes dynamic data like the trace id, span id, parent id.
Example implementations: node, python, go.

[Aaron Renner]

Would adding a new `Ecto.Query.comment/1` helper work?

MySchema

|> comment("My comment")

|> Repo.all()

This would basically generate "SELECT * FROM my_schema /* My comment */"

Then if we wanted to update all queries, we could override Ecto.Repo.prepare_query/3.

@impl true 

def prepare_query(_operation, query, opts) do

  comment_data = SQLCommenter.to_string(%{service: "my-service", parent_span_id: get_current_span_id()})

  comment(query, ^comment_data)

end

I know you mentioned ecto prepares the queries, but could we put dynamic data in them if we set the adapter's prepare option to unnamed? Being able to add sql comments would be really helpful for integrations with observability tools like datadog and cyral.

If this is ok, then I think we'd need to figure out if we'd probably need to design the API for multiple comments and figure out if we'd append or prepend them to the sql statement. Maybe there would be an option like

comment("My comment", placement: :prepend)

comment("My comment", placement: :append)

I'm curious what you think of this.

[Daniel Kukuła]

Sorry for the late reply.
This would be useful but I'm not sure if this should be in ecto or postgrex - I tried to hack this into ecto and I failed. 
Adding it to postgrex was much easier because there is access to the final query after the cache.

It would be useful also for other purposes - currently when I look in aws on the performance insights it shows only the beginning of the query - and this is always the select part, this is annoying because every select is the same, the where that's at the end is the problematic part.