public inbox for [email protected]  
help / color / mirror / Atom feed
From: jian he <[email protected]>
To: [email protected]
Cc: Tatsuo Ishii <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Row pattern recognition
Date: Fri, 12 Jun 2026 18:52:52 +0800
Message-ID: <CACJufxG57=ddtbN=5RZCzhxWDYXvocKmB7NtZy+DoqZuhxb_DA@mail.gmail.com> (raw)
In-Reply-To: <CAAAe_zDYxq0d3exCDwvKncD0kaL2uehDir6HXo4r5DXMitKrSg@mail.gmail.com>
References: <CAAAe_zAZDuHSiVGvz9c6h=Pe=aN+FKZOrdNPfbTOk3XV+WFKYQ@mail.gmail.com>
	<CAAAe_zDz3z2Paidk3jHOm9S3eMVLoXRxK0Lyo=5i_9-EfSH7fA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CACJufxFnwdQSApt2vWwYCd0gtf+JjFDxT2hbxHi=+dhFJc+-1g@mail.gmail.com>
	<CAAAe_zATnkqsbLYDj8MJV1TriX9Wi0wShDg3nK3qYpiupKwhFA@mail.gmail.com>
	<CAAAe_zBL+J0AYmvmcJQT7Q-gp5aRH0deJ7SE7-N21g4hWExyJw@mail.gmail.com>
	<CACJufxHJFNBJ0vHCCLestWV5b7DF5e4VzfmovqGKBGgqg+rcGA@mail.gmail.com>
	<CAAAe_zBY0rrgf+tKXMUc-Y3nDDD69hddRBKopEKAZobhY=Cy-Q@mail.gmail.com>
	<CAAAe_zDYxq0d3exCDwvKncD0kaL2uehDir6HXo4r5DXMitKrSg@mail.gmail.com>

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



--
jian
https://www.enterprisedb.com/


Attachments:

  [application/octet-stream] v47-0003-refactor-volatile-expression-check.nocfbot (8.8K, 2-v47-0003-refactor-volatile-expression-check.nocfbot)
  download

  [application/octet-stream] v47-0002-refactor-gram.y.nocfbot (5.4K, 3-v47-0002-refactor-gram.y.nocfbot)
  download

  [application/octet-stream] v47-0006-refactor-transformDefineClause.nocfbot (8.2K, 4-v47-0006-refactor-transformDefineClause.nocfbot)
  download

  [application/octet-stream] v47-0004-refactor-validateRPRPatternVarCount.nocfbot (4.6K, 5-v47-0004-refactor-validateRPRPatternVarCount.nocfbot)
  download

  [application/octet-stream] v47-0005-refactor-transformDefineClause.nocfbot (6.6K, 6-v47-0005-refactor-transformDefineClause.nocfbot)
  download

  [application/octet-stream] v47-0001-replace-some-AST-to-clause.nocfbot (4.3K, 7-v47-0001-replace-some-AST-to-clause.nocfbot)
  download

view thread (118+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Row pattern recognition
  In-Reply-To: <CACJufxG57=ddtbN=5RZCzhxWDYXvocKmB7NtZy+DoqZuhxb_DA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox