| From: | surya poondla <suryapoondla4(at)gmail(dot)com> |
|---|---|
| To: | Baji Shaik <baji(dot)pgdev(at)gmail(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: two small tab-completion patches (ALTER CONSTRAINT, CHECK modifiers) |
| Date: | 2026-10-21 23:30:06 |
| Message-ID: | CAOVWO5pME+cfKB=tXQYgOXy+0OwymzwJrnb5uo+KjB_WZHvJUA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Baji,
I tested v2 on top of fded90eec3e. Both patches apply cleanly (the CHECK
one has to go on top of the ALTER CONSTRAINT one) and build
without warnings.
I tested psql through a pty and compared master against patched for about
50 inputs:
- ALTER TABLE t ALTER CONSTRAINT fk <tab> now offers DEFERRABLE,
ENFORCED, INHERIT, INITIALLY DEFERRED/IMMEDIATE, NO INHERIT,
NOT DEFERRABLE, NOT ENFORCED; and NOT/NO/INITIALLY each complete to the
matching second word, so Chao's point is addressed.
- ALTER TABLE t ADD [CONSTRAINT c] CHECK (...) <tab> offers NO
INHERIT, NOT ENFORCED, NOT VALID, and NOT/NO follow up correctly.
This also works with nested parentheses in the expression and with
CHECK(a>0) written without a space.
- Lower-case input, a schema-qualified table and a quoted constraint name
all behave the same.
- The ALTER TABLE rules (ALTER [COLUMN] x ..., ALTER
CONSTRAINT <tab> listing names, DROP/RENAME/VALIDATE CONSTRAINT,
ADD, ADD CONSTRAINT x, ADD [CONSTRAINT x] NOT NULL, PRIMARY
KEY/UNIQUE, ADD c int) give identical output with and without the patches.
I also ran every suggested form against the server, all are accepted. NOT
VALID is correctly left out of the ALTER CONSTRAINT list, since
the grammar rejects it there. src/bin/psql/t/010_tab_completion.pl passes.
Two small, optional suggestions:
1. After "ALTER CONSTRAINT fk DEFERRABLE" (or NOT DEFERRABLE)
nothing further is offered, although "DEFERRABLE INITIALLY DEFERRED" is
probably the most common way to write it. A rule
offering INITIALLY DEFERRED / INITIALLY IMMEDIATE there would
be nice, but I'm fine leaving it out to keep the patch small.
2. A couple of cases in 010_tab_completion.pl, e.g. for "alter table tab1
alter constraint c not<tab>" and
"... check (a > 0) no<tab>", would keep the new rules from silently
regressing.
Neither is a blocker, v2 looks good to me as is.
Regards,
Surya Poondla
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Henson Choi | 2026-10-21 23:58:22 | Re: [DEPARSE] EXPLAIN prints another ModifyTable's RETURNING aliases in a SubPlan |
| Previous Message | Dean Rasheed | 2026-10-21 21:56:04 | Re: [DEPARSE] EXPLAIN prints another ModifyTable's RETURNING aliases in a SubPlan |