Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move queries with variable number of args to a fixed number of args syntax #1114

Merged
merged 8 commits into from
Nov 1, 2023

Conversation

ftkg
Copy link
Contributor

@ftkg ftkg commented Oct 23, 2023

Queries with a variable number of arguments are treated as distinct queries from PostgreSQL perspective, and their statistics are tracked separately. This PR aims to improve query metrics' cardinality and, consequently, improve query stats monitoring. This is achieved by using a fixed syntax:

  • WHERE x in ($1,$2,$3, ...) is replaced with WHERE x = ANY($1::type_of_x[]), with all values passed as a single array argument.
  • INSERT INTO t (a,b,c) VALUES ($1,$2,$3), ($4,$5,$6), ... is replaced with pgx.Batch, with a single row insert per query.

@ftkg ftkg requested a review from redbaron October 23, 2023 21:07
server/core_wallet.go Outdated Show resolved Hide resolved
@ftkg
Copy link
Contributor Author

ftkg commented Oct 26, 2023

Replaced pgx.Batch with INSERT INTO .. SELECT unnest() to avoid having to sort input data (due to deadlock potential). The single statement may also be more efficient.

@redbaron
Copy link
Contributor

Replaced pgx.Batch with INSERT INTO .. SELECT unnest() to avoid having to sort input data (due to deadlock potential). The single statement may also be more efficient.

Queries in Batch are executed in the same order they were added to Batch, which is in turn the same as in unnest, so deadlock potential should be the same.

As for efficiency, pgx uses prepared statement underndeath, so any request after first will be sending just args for the query, not the query itself. pgx will spend time hashinb query string (to match prepared statement), but I think simplicity of Batch makes it more appealing rather than fairly unconventional unnest .

If string hashing becomes a problem, then explicit prepared statement can be used with Batch.

@ftkg
Copy link
Contributor Author

ftkg commented Oct 26, 2023

Replaced pgx.Batch with INSERT INTO .. SELECT unnest() to avoid having to sort input data (due to deadlock potential). The single statement may also be more efficient.

Queries in Batch are executed in the same order they were added to Batch, which is in turn the same as in unnest, so deadlock potential should be the same.

As for efficiency, pgx uses prepared statement underndeath, so any request after first will be sending just args for the query, not the query itself. pgx will spend time hashinb query string (to match prepared statement), but I think simplicity of Batch makes it more appealing rather than fairly unconventional unnest .

If string hashing becomes a problem, then explicit prepared statement can be used with Batch.

The only other difference I see is tracking stats for every single query (Batch) vs the single bulk query (unnest) but I don't really prefer one or the other, wdyt?

@ftkg ftkg merged commit 211f6d1 into master Nov 1, 2023
3 checks passed
@ftkg ftkg deleted the ft-remove-variable-query-args branch November 1, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants