Re: Pass ParseState as down to utility functions.

On Tue, Dec 10, 2024 at 10:38:41PM +0800, jian he wrote:
> add parser_errposition to some places in
> transformTableConstraint, transformColumnDefinition
> where v8 didn't.

I've looked at the new tests in 0001. Here are some notes. And I've
found some mistakes and simplifications on the way.

CREATE TYPE test_type2 AS (a int, b text);
+CREATE TABLE test_tbl2 OF xx;
+ERROR: type "xx" does not exist
[...]
+ALTER TABLE tt0 OF tt_t_noexist;
+ERROR: type "tt_t_noexist" does not exist

typed_table.out checks that already, so these additions bring nothing
new:
CREATE TABLE ttable1 OF nothing;
ERROR: type "nothing" does not exist

The three tests for ALTER TABLE .. SET DATA TYPE are new patterns, so
these are OK. The COLLATE case was kind of covered with CREATE
DOMAIN, but the command is different.

The ALTER TABLE .. ALTER COLUMN case for a generated column is new, so
that's OK.

CREATE TYPE (like=no_such_type) also makes sense, that's new coverage.
This query pattern is only used in expressions, float8 and float4.

+--test error report position

As of 0001, this comment is incorrect. We're not testing an error
position yet. With 0002, it would be correct. Let's just use a more
generic wording that applies to both patches.

+create domain d_fail as int4 constraint cc generated always as (2) stored;
+ERROR: specifying GENERATED not supported for domains
+create domain d_fail as int4 constraint cc check(values > 1) no inherit;
+ERROR: check constraints for domains cannot be marked NO INHERIT

Can be reduced to one rather than two.

+create domain d_fail as int4 constraint cc check(values > 1) deferrable;
+ERROR: specifying constraint deferrability not supported for domains
+create domain d_fail as int4 constraint cc check(values > 1) not deferrable;
+ERROR: specifying constraint deferrability not supported for domains
+create domain d_fail as int4 constraint cc check (value > 1) initially deferred;
+ERROR: specifying constraint deferrability not supported for domains
+create domain d_fail as int4 constraint cc check(values > 1) initially immediate;
+ERROR: specifying constraint deferrability not supported for domains
+create domain d_fail as int4 constraint cc check(values > 1) deferrable not deferrable ;
+ERROR: specifying constraint deferrability not supported for domains

Testing the full set of keywords is not really interesting. So let's
just use one to make the script cheaper.

I was wondering for a few seconds about exclusion constraints, but it
requires a named constraint to trigger the error, so I've left it out
for now.

+create domain d_fail as int constraint cc REFERENCES this_table_not_exists(i);

Hmm. Funny. We don't document REFERENCES in the docs of CREATE
DOMAIN. Neither do we document GENERATED. Looks like a doc issue to
me, independent of this thread. ALTER DOMAIN uses a different parsing
clause than CREATE DOMAIN, meaning that generated columns or
references cannot be altered.. It looks like there's quite a bit more
going on here. The fact that we don't have tests for these patterns
authorized by the parser should be tracked anyway, so let's add them
for now. This should be looked at on a separate thread.

For now, I've applied the new tests. Let's move on with the additions
of 0002, and see if these are good to have or not (noticed Tom's
comments about the type paths, of course).
--
Michael