Re: Row pattern recognition

From: Henson Choi <assam258(at)gmail(dot)com>
To: jian he <jian(dot)universality(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-04 12:15:40
Message-ID: CAAAe_zBH-HtmPoTUPEsvozjyLptvX_rTRMOaPbyTnptj+mXoPw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Please check the attached regression test refactoring and gram.y changes

Thanks for the patch -- the substance all looks right: the gram.y reformat
is a fair consistency fix, the ecb2508aaf convention (no hardcoded error
text in regress comments) is one we should follow, and the added coverage
(split-token quantifier errors, SRF in DEFINE) fills real gaps.

I'll work these in together with the patches I'm lining up right before the
v48 merge, rather than mid-stream -- the large "Expected: ERROR" comment
cleanup would otherwise churn against the other test edits still in flight.
They'll all land in that v48 round.

Answers to your three questions:

> In src/test/regress/sql/rpr_base.sql, wording such as ``Jacob's
> Patterns`` should be removed?

Those came in from Jacob's branch, but agreed they're better changed:
"Jacob's Patterns" / "from jacob branch" are dev-time provenance notes, not
something to keep in committed tests. I'll give the section a descriptive
header (the tests themselves stay) and do it together with the v48 round
above.

> -- Serialization/Deserialization Tests (objects kept for
pg_upgrade/pg_dump)
> I am not sure what this refers to.

Those are views (with RPR in their definition) left undropped on purpose,
so the pg_dump/pg_upgrade tests pick them up: dumping and restoring the
regression DB deparses the RPR window clause and re-parses it, exercising
the serialization round trip. rpr_explain.sql keeps one for the same
reason. I'll reword the comment to say that.

> errmsg("quantifier bound must be between 0 and %d", INT_MAX - 1),
> errmsg("quantifier bound must be between 1 and %d", INT_MAX - 1),
> Will these cause consistency issues?

No, it's intentional. INT_MAX is the unbounded sentinel (RPR_QUANTITY_INF),
so an explicit bound is capped at INT_MAX - 1 to avoid colliding with it;
the "0 vs 1" split just matches each form ({n}/{,m} need >= 1, {n,} allows
0).

Regards,
Henson

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Imran Zaheer 2026-06-04 12:22:08 Move system identifier generation to a common helper
Previous Message Nazir Bilal Yavuz 2026-06-04 12:13:14 Re: Heads Up: cirrus-ci is shutting down June 1st