> 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
