Make queries friendly to prepared statements cache#317
Open
dominiquelefevre wants to merge 3 commits intogo-gorm:masterfrom
Open
Make queries friendly to prepared statements cache#317dominiquelefevre wants to merge 3 commits intogo-gorm:masterfrom
dominiquelefevre wants to merge 3 commits intogo-gorm:masterfrom
Conversation
Postgres has two extensions to ANSI SQL. One can write `col = ANY(?)`
instead of `col IN (?)`, and `col != ALL(?)` instead of `col NOT IN (?)`.
Semantically, the two forms are the same. The difference between them
lies in their interaction with prepared statements:
1. A condition .Where("col IN (?)", values) expands to `col IN ($1,$2,...)`
where the list has len(values) items. Every value[i] is sent to postgres
as a separate query argument.
2. A condition .Where("col = ANY(?)", values) expands to `col = ANY($1)`, and
values are sent to postgres as exactly one query argument (of an array type).
The option 1 does not iteract well with prepared statements. It produces
a different query for different len(values). which overflows the cache
of prepared statements quickly. Option 2, on the other hand, needs
only one prepared statement for any len(values).
This patch does not seek to optimise all instanceof of `col IN (?)`
because GORM does not really parse SQL clauses. It handles two cases
that I believe to be the most frequent:
1. .Where("col IN (?)", values) and .Where("col NOT IN (?)", values),
2. .Where(map[string]any{"col": values}).
An INSERT INTO that creates N rows is expanded into the following
query:
INSERT INTO table(...) VALUES ($1,$2,...), ($K,$K+1,...),...
------------------------------
N tuples
This way, we get a unique query for every value of N. This overflows
the cache of prepared statements quickly. Just do not use prepared
statements when doing bulk inserts.
Postgres has a better solution with its COPY protocol. It allows us
to use one prepared statement for N = 1 to insert multiple values.
Unfortunately, I have found no good way to use the COPY protocol
when an insert has an ON CONFLICT clause. Let us stick with a simpler
solution for now.
Author
|
Hey @jinzhu could you please have a look at the patches? |
Author
|
Hey @jinzhu is this repo dead? |
Member
|
Sorry for the delay. I’ve reviewed this PR, but I think there might be a better way to handle it. Unfortunately, I’ve been too busy recently to provide detailed suggestions. My current thought is that we should consider extending the AddVar method of Statement and the BindVarTo method of the dialector. I’m still thinking about the best approach to do this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes two scenarios where GORM quickly overflows pgx's cache of prepared statements and degrades into a mode where every DB request has to do two round-trips for Prepare() + ExecPrepared() instead of just ExecPrepared().
See commit messages for details about the scenarios and their fixes.