public inbox for [email protected]  

help / color / mirror / Atom feed

From: Henson Choi <[email protected]>
To: jian he <[email protected]>
To: 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: Thu, 18 Jun 2026 16:48:54 +0900
Message-ID: <CAAAe_zApTOBLS9TCDM=YEvjTEBLa84gzRKspqTQSw6NK9iB+Ug@mail.gmail.com> (raw)
In-Reply-To: <CACJufxG6rKfd0WtsVZgzcoh8vOEwo_8UDWkuOd888ATavNv_uw@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>
	<CACJufxG57=ddtbN=5RZCzhxWDYXvocKmB7NtZy+DoqZuhxb_DA@mail.gmail.com>
	<CAAAe_zB4TMHnpaOrwYp7dKs553q2474ZXRytGfYOfYp4DdrgiQ@mail.gmail.com>
	<CACJufxFAQhbOD9EVCTAy-VwDbG4446N10GsxCcgdpFnjHO1Efw@mail.gmail.com>
	<CACJufxG6rKfd0WtsVZgzcoh8vOEwo_8UDWkuOd888ATavNv_uw@mail.gmail.com>

Hi Jian,

Thanks for the misc-refactoring patch (v48-0001-v48-misc-refactoring) and
the
whole-row Var note.  As before, now that v48 is posted I'll rebase on it
and send
these as incremental patches on top (v49); per-item disposition below, with
the
per-commit breakdown to come with the patch.


> RPRNavExpr->resulttype should also marked as
pg_node_attr(query_jumble_ignore)

Agreed -- every other derived result-type field in primnodes.h already
carries it,
and resulttype is derived from the (jumbled) arg type, so ignoring it loses
nothing.
I'll add it.


> collectPatternVariables is not needed.
> The parser already ensures every DEFINE variable appears in PATTERN ...
> See nfa_evaluate_row the for loop break.

Confirmed -- the parser already rejects a DEFINE variable not used in
PATTERN, so
every DEFINE var is in PATTERN; filtering is redundant and cost_windowagg
can
iterate defineClause directly, so I'll remove it.


> buildDefineVariableList is trivial. No need to export it as an external
function.

Agreed -- I'll drop it and inline the list-building into
create_windowagg_plan().


> Rename WindowAggState.defineClauseList to defineClauseExprs

Agreed (the elements are ExprState, so it matches the ...Exprs convention)
-- I'll
do it, including the additional site the dormant-match fix touches.


> Flatten a needlessly nested block in show_window_def().
> Replace a post-loop ListCell NULL check in
remove_unused_subquery_outputs()
> with a boolean flag.
> Reduce the number of arguments in make_windowagg.
> Minor refactoring of regress test comments.

I'll do all of these.

One exception: the parsenodes.h RPRPatternNode comment names the data
structure, so
rather than "clause" I'll change it to "parse tree node".


> dvar, var both can be whole-row Vars, but this seems to work for
whole-row vars.
> We need some simple regress tests for cases where both are whole-row vars
or one
> of them is a whole-row var.

Good catch -- I'll add regress coverage for the whole-row Var cases (both
sides
whole-row, and one side whole-row) in the needed_by_define check, in v49.

I'll send the items above as a first incremental patch on top of v48, and
take up
the later reviews after that.


Thanks,
Henson

view thread (129+ 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: <CAAAe_zApTOBLS9TCDM=YEvjTEBLa84gzRKspqTQSw6NK9iB+Ug@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