add tab-complete for ALTER DOMAIN ADD...
hi.
per https://www.postgresql.org/docs/current/sql-alterdomain.html
we can add tab-complete for ALTER DOMAIN ADD variants:
ALTER DOMAIN sth ADD CHECK
ALTER DOMAIN sth ADD CONSTRAINT
ALTER DOMAIN sth ADD NOT NULL
Attachments:
v1-0001-add-tab-complete-for-ALTER-DOMAIN-ADD.patchapplication/octet-stream; name=v1-0001-add-tab-complete-for-ALTER-DOMAIN-ADD.patchDownload
From cac643589c96a8caf1be0bab5aba1f3bc89fcf66 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Tue, 29 Apr 2025 18:28:49 +0800
Subject: [PATCH v1 1/1] add tab-complete for ALTER DOMAIN ADD ...
per https://www.postgresql.org/docs/current/sql-alterdomain.html
we can add tab-complete for ALTER DOMAIN ADD:
ALTER DOMAIN ADD CHECK
ALTER DOMAIN ADD CONSTRAINT
ALTER DOMAIN ADD NOT NULL
discussion: https://postgr.es/m
---
src/bin/psql/tab-complete.in.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index c916b9299a8..e8ff574b0e4 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2539,6 +2539,9 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "DOMAIN", MatchAny))
COMPLETE_WITH("ADD", "DROP", "OWNER TO", "RENAME", "SET",
"VALIDATE CONSTRAINT");
+ /* ALTER DOMAIN <sth> ADD */
+ else if (Matches("ALTER", "DOMAIN", MatchAny, "ADD"))
+ COMPLETE_WITH("CONSTRAINT", "NOT NULL", "CHECK");
/* ALTER DOMAIN <sth> DROP */
else if (Matches("ALTER", "DOMAIN", MatchAny, "DROP"))
COMPLETE_WITH("CONSTRAINT", "DEFAULT", "NOT NULL");
--
2.39.5 (Apple Git-154)
jian he <jian.universality@gmail.com> writes:
hi.
per https://www.postgresql.org/docs/current/sql-alterdomain.html
we can add tab-complete for ALTER DOMAIN ADD variants:
ALTER DOMAIN sth ADD CHECK
ALTER DOMAIN sth ADD CONSTRAINT
ALTER DOMAIN sth ADD NOT NULL
Good catch.
+ /* ALTER DOMAIN <sth> ADD */ + else if (Matches("ALTER", "DOMAIN", MatchAny, "ADD")) + COMPLETE_WITH("CONSTRAINT", "NOT NULL", "CHECK");
I think the completion for CHECK should include the opening paren too,
since that's required for the expression. We could also add completion
after CONSTRAINT <name>, like this:
else if(Matches("ALTER", "DOMAIN", MatchAny, "ADD", "CONSTRAINT", MatchAny))
COMPLETE_WITH("NOT NULL", "CHECK (");
- ilmari
On 2025-Apr-29, Dagfinn Ilmari Mannsåker wrote:
jian he <jian.universality@gmail.com> writes:
+ /* ALTER DOMAIN <sth> ADD */ + else if (Matches("ALTER", "DOMAIN", MatchAny, "ADD")) + COMPLETE_WITH("CONSTRAINT", "NOT NULL", "CHECK");I think the completion for CHECK should include the opening paren too,
since that's required for the expression.
Yeah, we do that elsewhere.
We could also add completion after CONSTRAINT <name>, like this:
else if(Matches("ALTER", "DOMAIN", MatchAny, "ADD", "CONSTRAINT", MatchAny))
COMPLETE_WITH("NOT NULL", "CHECK (");
Done that, and pushed.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Los dioses no protegen a los insensatos. Éstos reciben protección de
otros insensatos mejor dotados" (Luis Wu, Mundo Anillo)
So we should also have tab-completion for ALTER TABLE ADD NOT NULL, as
in the attached. Any opinions?
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Estoy de acuerdo contigo en que la verdad absoluta no existe...
El problema es que la mentira sí existe y tu estás mintiendo" (G. Lama)
Attachments:
0001-Add-tab-completion-for-ALTER-TABLE-not-nulls.patchtext/x-diff; charset=utf-8Download
From 526f816542b45773a6bb2214c8c23a15075e4b9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Sun, 11 May 2025 10:45:49 -0400
Subject: [PATCH] Add tab-completion for ALTER TABLE not-nulls
Command is: ALTER TABLE x ADD [CONSTRAINT y] NOT NULL z
---
src/bin/psql/tab-complete.in.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index ec65ab79fec..a6135f4b6cb 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2725,17 +2725,24 @@ match_previous_words(int pattern_id,
/* ALTER TABLE xxx ADD */
else if (Matches("ALTER", "TABLE", MatchAny, "ADD"))
{
- /* make sure to keep this list and the !Matches() below in sync */
+ /*
+ * make sure to keep this list and the MatchAnyExcept() below in sync
+ */
COMPLETE_WITH("COLUMN", "CONSTRAINT", "CHECK", "UNIQUE", "PRIMARY KEY",
- "EXCLUDE", "FOREIGN KEY");
+ "NOT NULL", "EXCLUDE", "FOREIGN KEY");
}
/* ALTER TABLE xxx ADD [COLUMN] yyy */
else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN", MatchAny) ||
- Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAnyExcept("COLUMN|CONSTRAINT|CHECK|UNIQUE|PRIMARY|EXCLUDE|FOREIGN")))
+ Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAnyExcept("COLUMN|CONSTRAINT|CHECK|UNIQUE|PRIMARY|NOT|EXCLUDE|FOREIGN")))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes);
/* ALTER TABLE xxx ADD CONSTRAINT yyy */
else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny))
- COMPLETE_WITH("CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY");
+ COMPLETE_WITH("CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY", "NOT NULL");
+ /* ALTER TABLE xxx ADD NOT NULL */
+ else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "NOT", "NULL"))
+ COMPLETE_WITH_ATTR(prev4_wd);
+ else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "NOT", "NULL"))
+ COMPLETE_WITH_ATTR(prev6_wd);
/* ALTER TABLE xxx ADD [CONSTRAINT yyy] (PRIMARY KEY|UNIQUE) */
else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "PRIMARY", "KEY") ||
Matches("ALTER", "TABLE", MatchAny, "ADD", "UNIQUE") ||
--
2.39.5
On Sun, 11 May 2025, at 15:48, Álvaro Herrera wrote:
So we should also have tab-completion for ALTER TABLE ADD NOT NULL, as
in the attached. Any opinions?
LGTM
--
- ilmari
On 2025/05/11 23:22, Álvaro Herrera wrote:
On 2025-Apr-29, Dagfinn Ilmari Mannsåker wrote:
jian he <jian.universality@gmail.com> writes:
+ /* ALTER DOMAIN <sth> ADD */ + else if (Matches("ALTER", "DOMAIN", MatchAny, "ADD")) + COMPLETE_WITH("CONSTRAINT", "NOT NULL", "CHECK");I think the completion for CHECK should include the opening paren too,
since that's required for the expression.Yeah, we do that elsewhere.
We could also add completion after CONSTRAINT <name>, like this:
else if(Matches("ALTER", "DOMAIN", MatchAny, "ADD", "CONSTRAINT", MatchAny))
COMPLETE_WITH("NOT NULL", "CHECK (");Done that, and pushed.
I have no objection to this commit. However, I had assumed we would
wait to commit changes like this - which aren't bug fixes or
v18-related oversights - until master becomes the development branch
for v19. Maybe I'm missing something..
We can go ahead with this one because it's a small change? Just checking,
since I have a few similar tab-completion improvements patches and
have been holding off until v19 development begins. If it's fine,
I'm thinking of committing them soon as well.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
On 2025/05/11 23:22, Álvaro Herrera wrote:
Done that, and pushed.
I have no objection to this commit. However, I had assumed we would
wait to commit changes like this - which aren't bug fixes or
v18-related oversights - until master becomes the development branch
for v19. Maybe I'm missing something..
I was surprised by this commit too. Yeah, it's small, but feature
freeze is not about small.
regards, tom lane
On 2025-May-12, Fujii Masao wrote:
I have no objection to this commit. However, I had assumed we would
wait to commit changes like this - which aren't bug fixes or
v18-related oversights - until master becomes the development branch
for v19. Maybe I'm missing something..
Yeah, fair question. We got new syntax to add NOT NULL constraints, and
this adds tab-completion for that. It does, in addition, add
tab-completion for CHECK constraints and for the CONSTRAINT keyword,
which is much older syntax. So I guess you could argue that it would
have been okay to add the NOT NULL one (it could be considered an open
item), but not the the other two because they're instead a "new
feature". But that helps nobody, because we would be offering _some_
options in a command but not all possible ones, which I think is even
worse because it'd be misleading.
Consider the other patch I sent later. It adds tab-completion coverage
for the new NOT NULL syntax in ALTER TABLE, so it should be fair game
for 18, shouldn't it? Again: it'd be misleading not to tab-complete
that syntax, because it would confuse people into failing to realize
that that syntax exists, if they see other options but not that one.
As for whether your proposed tab-completion improvements are acceptable
to be included now, I guess it's all subjective on what exactly they
are.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"I think my standards have lowered enough that now I think 'good design'
is when the page doesn't irritate the living f*ck out of me." (JWZ)
On 2025-May-11, Álvaro Herrera wrote:
So we should also have tab-completion for ALTER TABLE ADD NOT NULL, as
in the attached. Any opinions?
I have pushed this now, adding the parens after CHECK and reordering the
options per Fujii's suggestion in another thread. I opted to push only
to master, but there's room to argue that this should be backpatched to
18 since it covers syntax newly added there. If I get a couple of ayes
and not too many nays, I would do so at some point before beta2.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"El que vive para el futuro es un iluso, y el que vive para el pasado,
un imbécil" (Luis Adler, "Los tripulantes de la noche")