elixir-ecto / db_connection

Database connection behaviour
http://hexdocs.pm/db_connection/DBConnection.html
306 stars 113 forks source link

Automatically reprepare query on EncodeError #141

Closed josevalim closed 6 years ago

josevalim commented 6 years ago

Closes #123

josevalim commented 6 years ago

Btw, this patch does not work for Postgrex as is. If we attempt to prepare an already prepared query, it will error. So we need to do something, either by augmenting the query protocol or by passing an option to make this work.

However, even by doing those changes, this is not enough to fix Ecto tests on db_connection (there are 2 and one is related to this, you can see on CI). --

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

fishcakez commented 6 years ago

Yes we would need to move the logic in postgrex that prevents a query thats already prepared to error on parse/2 and to unprepare the query in the postgrex protocol if we try to prepare the prepared query. The postgrex protocol module should definitely handle this gracefully (close and reprepare) with existing logi.

josevalim commented 6 years ago

So it seems this patch is wrong? We should not call parse again when falling back from execute to prepare_execute? --

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

fishcakez commented 6 years ago

Correct don't need to parse when it's already been done.

On 2 Sep 2018 00:30, "José Valim" notifications@github.com wrote:

So it seems this patch is wrong? We should not call parse again when falling back from execute to prepare_execute?

--

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/elixir-ecto/db_connection/pull/141#issuecomment-417911026, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6JTYc6u6y4ddKbhZihUk2By5xSOjbvks5uW4krgaJpZM4WWKJZ .

josevalim commented 6 years ago

I have fixed the typo, changed it so we don't parse twice and moved to {:prepare, meter} as return. It should be good to go. I will try it with Postgrex master soon.

josevalim commented 6 years ago

:heart: :green_heart: :blue_heart: :yellow_heart: :purple_heart: