Repository navigation
Fix greedy comment regex and deallocate-all blast radius (stacked on #14) - #15
Closed
utkarash2991 wants to merge 1 commit into
Conversation
utkarash2991
force-pushed
the
utkarashsingh/psqlext-1773-fix-comment-regex-and-deallocate
branch
from
October 6, 2026 10:47
3c6b3f7 to
b835380
Compare
Two issues found during review of the function, both silent and both
real:
1. The block-comment regex ('\/\*.+\*\/') is greedy across every /* */
pair in the query, not just one. A query with two separate block
comments had everything between the first "/*" and the last "*/"
deleted - including real code in between - with no error raised.
Verified: regexp_replace('select /* c1 */ a /* c2 */ from t',
'\/\*.+\*\/', '', 'g') returns 'select from t' - the column "a" is
gone. Fixed with a non-greedy quantifier.
2. The two bare "deallocate all;" calls wipe every prepared statement in
the calling session, not just index_advisor's own internal one.
Verified: preparing an unrelated statement, then calling
index_advisor(), left pg_prepared_statements with 0 rows. In a pooled
or persistent connection (PostgREST, pgbouncer session mode) this
silently breaks whatever else that connection had prepared. Fixed by
checking pg_prepared_statements for the specific name before
deallocating it - DEALLOCATE has no IF EXISTS clause, so this needs an
explicit existence check rather than DROP-style IF EXISTS syntax
(caught by the new test on the first attempt).
Ships as 0.2.2, as an incremental upgrade script on top of 0.2.1.
utkarash2991
force-pushed
the
utkarashsingh/psqlext-1773-fix-comment-regex-and-deallocate
branch
from
October 6, 2026 10:51
b835380 to
19ab033
Compare
hanefi
reviewed
Oct 6, 2026
| extversion | ||
| ------------ | ||
| 0.2.1 | ||
| 0.2.2 |
There was a problem hiding this comment.
IMO it does not make sense to bump extension version on all PRs. How about keeping 0.2.1 as it is not released yet?
An error occurred while trying to automatically change base from
utkarashsingh/psqlext-1773-schema-qualify-hypopg-calls-v2
to
main
October 8, 2026 14:55
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.
Stacked on #14
Targets
utkarashsingh/psqlext-1773-schema-qualify-hypopg-calls-v2(#14), notmain- merge #13 and #14 first.Two issues found reviewing the function while working on PSQLEXT-1773, both silent (no error raised) and both verified before and after the fix:
1. Greedy block-comment regex deletes code between separate comments
The comment-stripping regex (
'\/\*.+\*\/') is greedy across every/* */pair in the query, not just the nearest one. A query with two separate block comments has everything between the first/*and the last*/deleted - including real code in between.Fixed with a non-greedy quantifier (
\/\*.+?\*\/).2.
deallocate allwipes every prepared statement in the sessionThe two bare
deallocate all;calls clear every prepared statement in the calling session, not justindex_advisor's own internal one. In a pooled or persistent connection (PostgREST, pgbouncer session mode) this silently breaks whatever else that connection had prepared.Verified: preparing an unrelated statement, then calling
index_advisor(), leftpg_prepared_statementswith 0 rows afterward.Fixed by checking
pg_prepared_statementsfor the specific name before deallocating it.Tracked in https://linear.app/supabase/issue/PSQLEXT-1773