-
Notifications
You must be signed in to change notification settings - Fork 59
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
PG -156: replace query placeholders with actual arguments for prepared statements #481
Merged
Conversation
This file contains 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
darkfronza
force-pushed
the
PG-156-replace-query-placeholders
branch
2 times, most recently
from
August 9, 2024 19:28
627e075
to
2487f44
Compare
darkfronza
force-pushed
the
PG-156-replace-query-placeholders
branch
from
August 9, 2024 19:36
2487f44
to
3d0a3cf
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #481 +/- ##
==========================================
+ Coverage 84.88% 85.38% +0.50%
==========================================
Files 3 3
Lines 1297 1362 +65
Branches 198 208 +10
==========================================
+ Hits 1101 1163 +62
+ Misses 98 97 -1
- Partials 98 102 +4 ☔ View full report in Codecov by Sentry. |
artemgavrilov
approved these changes
Aug 19, 2024
darkfronza
force-pushed
the
PG-156-replace-query-placeholders
branch
2 times, most recently
from
August 19, 2024 20:47
4223d61
to
97535ef
Compare
Added support for extracting query arguments for prepared statements when `pg_stat_monitor.pgsm_normalized_query` is off. Previously pg_stat_monitor was unable to extract the arguments for prepared statements, thus leaving queries with placeholders $1 .. $N instead of the actual arguments.
Instead of copying original query text byte by byte, copy data between query placeholders in chunks, example: `INSERT INTO foo(a, b, c) VALUES('test', 100, 'test again)'` Would result in normalized query: `INSERT INTO foo(a, b, c) VALUES($1, $2, $3)` The original patch would copy the parts between placeholders byte by byte, e.g. `INSERT INTO foo(a, b, c) VALUES(`, instead we can copy this whole block at once, 1 function call and maybe 1 buffer re-allocation per call. Also make use of `appendBinaryStringInfo` to avoid calculating string length as we have this info already.
Avoid allocating an array of strings for extracting query argument values, instead append the current parameter value directly in the buffer used to store the denormalized query. This avoids not only unnecessary memory allocations, but also copying data between temporary memory and the buffer.
This commit introduces a little optimization along with a feature, it stores the query in denormalized form only under the circumstances below: - The psgm_normalized_query GUC is disabled (off). - The query is seem for the first time, or the query total execution time exceeds the mean execution time calculated for the previous queries. Having the query which took most execution time along with it's arguments could help users in further investigating performance issues.
When query normalization is disabled utility queries like SELECT 10+20 are now stored as is, instead of SELECT $1+$2. Also when functions or sub queries are created the arguments used internally by the function or subqueries will be replaced by NULL instead of $1..$N. The actual arguments will be displayed when the function or subquery is actually invoked.
Ensures that the denormalization of prepared statements is working, also ensure that a query which takes more time to execute replaces the previous denormalized query.
With the query dernomalization feature, having integer literals used in sql like 1, or 2 could create some confusion on whether those are placeholders or constant values, thus this commit updates the pgsm_query_id regression test to use different integer literals to avoid confusion.
Add a new test case: 1. Execute a prepared statement with larger execution time first. 2. Execute the same prepared statement with cheap execution time. 3. Ensures that the denormalized heavy query is not replaced by the cheaper.
On PG 12, 13, the internal return instruction in the following function: ``` CREATE OR REPLACE FUNCTION add(int, int) RETURNS INTEGER AS $$ BEGIN return (select $1 + $2); END; $$ language plpgsql; ``` Is stored as SELECT (select expr1 + expr2) On PG 14 onward it's stored just as SELECT (expr1 + expr2)
darkfronza
force-pushed
the
PG-156-replace-query-placeholders
branch
from
August 26, 2024 20:31
97535ef
to
43d5548
Compare
codeforall
approved these changes
Nov 1, 2024
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.
PG-156
Before this PR,
pg_stat_monitor
was unable to replace query placeholders when tracking prepared statements ifpg_stat_monitor.pgsm_normalized_query
isoff
.With this update
pg_stat_monitor
now correctly store query argument values for prepared statements.