search

elixir-ecto / ecto_sql

SQL-based adapters for Ecto and database migrations
https://hexdocs.pm/ecto_sql
Apache License 2.0
578 stars 312 forks source link

Add pg 16 to CI and remove `:generic` explain plan option #606

Closed greg-rychlewski closed 6 months ago

greg-rychlewski commented 6 months ago

I got PG 16 to work on the CI. The issue is we use the image tags 16.0, 11.1, etc. As of Postgres 16 this tries to get the debian image instead of alpine. Appending -alpine to the tag name fixes it. I added it to all the old ones as well so that no one wonders why some have -alpine and some don't, in the future.

At the same time, adding these tests uncovered an issue supporting Postgres's built-in generic option. The issue is we have to send a query with placeholders without sending the actual parameter values. For example we have to send

EXPLAIN (generic_plan) select * from table where field = $1

But if we send the parameter values they will be used in the explain plan. If we don't send the parameter values then we get into issues with Postgrex because it checks to make sure we have the right number of params. We get an error like this

(ArgumentError) parameters must be of length 2 for query %Postgrex.Query{...

I'm not really convinced it's worth digging into the guts of Postgrex just for this one explain option when we already have a workaround. And even if we could alter Postgrex for this case, I'm not sure Postgres will work as we expect. So I am suggesting to remove that option.