| From: | jian he <jian(dot)universality(at)gmail(dot)com> |
|---|---|
| To: | assam258(at)gmail(dot)com |
| Cc: | Tatsuo Ishii <ishii(at)postgresql(dot)org>, zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, vik(at)postgresfriends(dot)org, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2026-06-12 10:52:52 |
| Message-ID: | CACJufxG57=ddtbN=5RZCzhxWDYXvocKmB7NtZy+DoqZuhxb_DA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi.
Based on https://github.com/assam258-5892/postgres/commits/RPR
Here are more minor review comments:
v47-0001-replace-some-AST-to-clause.nocfbot
Replace some "AST" to "clause".
v47-0002-refactor-gram.y.nocfbot
group Op and trailing ALT into one IF condition, this might improve readability.
v47-0003-refactor-volatile-expression-check.nocfbot
validate_rpr_define_volatility function, along with its helper
functions is removed.
contain_volatile_functions() already covers both volatile FuncExpr callees and
NextValueExpr, so a separate walker is not needed. The trade-off is that we lose
the error cursor position, but that seems better than to maintaining extra code.
The check is also moved to after preprocess_expression(), following the same
pattern as contain_volatile_functions_after_planning.
v47-0004-refactor-validateRPRPatternVarCount.nocfbot
validateRPRPatternVarCount() is recursive. Using the `rpDefs != NULL` sentinel
to gate the "DEFINE variable not used in PATTERN" check at the outermost call
level is awkward. That cross-check only needs to run once; it is better
expressed in the caller, transformDefineClause().
v47-0005-refactor-transformDefineClause.nocfbot
coerce_to_boolean() was deferred to a second pass over the completed
defineClause list, meaning pull_var_clause() ran on an expression that had not
yet been fully coerced/transformed. The more intuitively order should be:
transformExpr → coerce_to_boolean → pull_var_clause, so pull_var_clause always
sees the final expression form.
v47-0006-refactor-transformDefineClause.nocfbot
The allowed nav-expression combination is limited, therefore,
has_column_ref is not necessary.
Column reference checks can use contain_var_clause, and it's cheap.
Also, I made the following error message change:
-ERROR: row pattern navigation offset must be a run-time constant
+ERROR: row pattern navigation offset expression must not contain
column references
| Attachment | Content-Type | Size |
|---|---|---|
| v47-0003-refactor-volatile-expression-check.nocfbot | application/octet-stream | 8.8 KB |
| v47-0002-refactor-gram.y.nocfbot | application/octet-stream | 5.4 KB |
| v47-0006-refactor-transformDefineClause.nocfbot | application/octet-stream | 8.2 KB |
| v47-0004-refactor-validateRPRPatternVarCount.nocfbot | application/octet-stream | 4.6 KB |
| v47-0005-refactor-transformDefineClause.nocfbot | application/octet-stream | 6.6 KB |
| v47-0001-replace-some-AST-to-clause.nocfbot | application/octet-stream | 4.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-06-12 10:54:11 | Re: Fix race in ReplicationSlotRelease for ephemeral slots |
| Previous Message | solai v | 2026-06-12 10:46:14 | Re: Improving psql autocompletion for SET LOCAL / SET SESSION |