Thread: CREATE MATERIALIZED VIEW

CREATE MATERIALIZED VIEW

From
px shi
Date:
Hi, I see that materialized view cannot be unlogged now, but when I use psql and type CREATE UNLOGGED, pressing the Tab key for auto-completion suggests `TABLE` and MATERIALIZED VIEW. 
Shouldn't `MATERIALIZED VIEW ` be suggested?

Re: CREATE MATERIALIZED VIEW

From
Dagfinn Ilmari Mannsåker
Date:
px shi <spxlyy123@gmail.com> writes:

> Hi, I see that materialized view cannot be unlogged now, but when I use
> psql and type CREATE UNLOGGED, pressing the Tab key for auto-completion
> suggests `TABLE` and MATERIALIZED VIEW.
> Shouldn't `MATERIALIZED VIEW ` be suggested?

That's my fault, I added it in commit c951e9042dd1, presumably because
the grammar allows it, but it turns transformCreateTableAsStmt() rejects
it.

Attached is a patch to fix it, which sholud be backpatched to v17.

- ilmari

From f8bc4d760aadd5f1f3d805f6de7c0d6d3eb8d078 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Thu, 25 Jul 2024 14:33:14 +0100
Subject: [PATCH] Don't tab complete MATERIALIZED VIEW after CREATE UNLOGGED

This was erroneously added in commit c951e9042dd1 because the grammar
allows it, but it turns out transformCreateTableAsStmt() rejects it.
---
 src/bin/psql/tab-complete.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 891face1b6..024469474d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3267,15 +3267,9 @@ psql_completion(const char *text, int start, int end)
     /* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */
     else if (TailMatches("CREATE", "TEMP|TEMPORARY"))
         COMPLETE_WITH("SEQUENCE", "TABLE", "VIEW");
-    /* Complete "CREATE UNLOGGED" with TABLE, SEQUENCE or MATVIEW */
+    /* Complete "CREATE UNLOGGED" with TABLE or SEQUENCE */
     else if (TailMatches("CREATE", "UNLOGGED"))
-    {
-        /* but not MATVIEW in CREATE SCHEMA */
-        if (HeadMatches("CREATE", "SCHEMA"))
-            COMPLETE_WITH("TABLE", "SEQUENCE");
-        else
-            COMPLETE_WITH("TABLE", "SEQUENCE", "MATERIALIZED VIEW");
-    }
+        COMPLETE_WITH("TABLE", "SEQUENCE");
     /* Complete PARTITION BY with RANGE ( or LIST ( or ... */
     else if (TailMatches("PARTITION", "BY"))
         COMPLETE_WITH("RANGE (", "LIST (", "HASH (");
-- 
2.39.2


Re: CREATE MATERIALIZED VIEW

From
Dagfinn Ilmari Mannsåker
Date:
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:

> px shi <spxlyy123@gmail.com> writes:
>
>> Hi, I see that materialized view cannot be unlogged now, but when I use
>> psql and type CREATE UNLOGGED, pressing the Tab key for auto-completion
>> suggests `TABLE` and MATERIALIZED VIEW.
>> Shouldn't `MATERIALIZED VIEW ` be suggested?
>
> That's my fault, I added it in commit c951e9042dd1, presumably because
> the grammar allows it, but it turns transformCreateTableAsStmt() rejects
> it.

Scratch that, I misread the diff. The tab completion has been there
since matviews were added in commit 3bf3ab8c5636, but the restriction on
unlogged matviews was added later in commit 3223b25ff73, which failed to
update the tab completion code to match.

Here's a updated patch with a corrected commit message.

- ilmari

From 58aebeda0c2161598b58f468d3c46b113651fb79 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Thu, 25 Jul 2024 14:33:14 +0100
Subject: [PATCH v2] Don't tab complete MATERIALIZED VIEW after CREATE UNLOGGED

When materialized views were originally added in commit 3bf3ab8c5636,
they could be unlogged, so the tab completion was added. However, when
commit 3223b25ff73 banned them, the tab completion was not updated to
match.
---
 src/bin/psql/tab-complete.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 891face1b6..024469474d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3267,15 +3267,9 @@ psql_completion(const char *text, int start, int end)
     /* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */
     else if (TailMatches("CREATE", "TEMP|TEMPORARY"))
         COMPLETE_WITH("SEQUENCE", "TABLE", "VIEW");
-    /* Complete "CREATE UNLOGGED" with TABLE, SEQUENCE or MATVIEW */
+    /* Complete "CREATE UNLOGGED" with TABLE or SEQUENCE */
     else if (TailMatches("CREATE", "UNLOGGED"))
-    {
-        /* but not MATVIEW in CREATE SCHEMA */
-        if (HeadMatches("CREATE", "SCHEMA"))
-            COMPLETE_WITH("TABLE", "SEQUENCE");
-        else
-            COMPLETE_WITH("TABLE", "SEQUENCE", "MATERIALIZED VIEW");
-    }
+        COMPLETE_WITH("TABLE", "SEQUENCE");
     /* Complete PARTITION BY with RANGE ( or LIST ( or ... */
     else if (TailMatches("PARTITION", "BY"))
         COMPLETE_WITH("RANGE (", "LIST (", "HASH (");
-- 
2.39.2


Re: CREATE MATERIALIZED VIEW

From
Nathan Bossart
Date:
On Thu, Jul 25, 2024 at 03:09:09PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Scratch that, I misread the diff. The tab completion has been there
> since matviews were added in commit 3bf3ab8c5636, but the restriction on
> unlogged matviews was added later in commit 3223b25ff73, which failed to
> update the tab completion code to match.

As noted a few years ago [0], the commit message for 3223b25ff73 indicates
this was intentional:

    I left the grammar and tab-completion support for CREATE UNLOGGED
    MATERIALIZED VIEW in place, since it's harmless and allows delivering a
    more specific error message about the unsupported feature.

However, since it looks like the feature was never actually supported in a
release, and the revert has been in place for over a decade, I think it'd
be reasonable to remove the tab completion now.  It looks like the folks on
the 2021 thread felt similarly.

[0] https://postgr.es/m/flat/ZR0P278MB092093E92263DE16734208A5D2C59%40ZR0P278MB0920.CHEP278.PROD.OUTLOOK.COM

-- 
nathan



Re: CREATE MATERIALIZED VIEW

From
Dagfinn Ilmari Mannsåker
Date:
Nathan Bossart <nathandbossart@gmail.com> writes:

> On Thu, Jul 25, 2024 at 03:09:09PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Scratch that, I misread the diff. The tab completion has been there
>> since matviews were added in commit 3bf3ab8c5636, but the restriction on
>> unlogged matviews was added later in commit 3223b25ff73, which failed to
>> update the tab completion code to match.
>
> As noted a few years ago [0], the commit message for 3223b25ff73 indicates
> this was intentional:
>
>     I left the grammar and tab-completion support for CREATE UNLOGGED
>     MATERIALIZED VIEW in place, since it's harmless and allows delivering a
>     more specific error message about the unsupported feature.

D'oh, I'm clearly struggling with reading things properly today.

> However, since it looks like the feature was never actually supported in a
> release, and the revert has been in place for over a decade, I think it'd
> be reasonable to remove the tab completion now.  It looks like the folks on
> the 2021 thread felt similarly.

Keeping it in the grammar makes sense for the more specific error
message, but I don't think the tab completion should be suggesting bogus
things, regardless of whether it's the grammar or the parse analysis
that rejects it.

> [0] https://postgr.es/m/flat/ZR0P278MB092093E92263DE16734208A5D2C59%40ZR0P278MB0920.CHEP278.PROD.OUTLOOK.COM

- ilmari



Re: CREATE MATERIALIZED VIEW

From
Nathan Bossart
Date:
On Thu, Jul 25, 2024 at 03:49:02PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> However, since it looks like the feature was never actually supported in a
>> release, and the revert has been in place for over a decade, I think it'd
>> be reasonable to remove the tab completion now.  It looks like the folks on
>> the 2021 thread felt similarly.
> 
> Keeping it in the grammar makes sense for the more specific error
> message, but I don't think the tab completion should be suggesting bogus
> things, regardless of whether it's the grammar or the parse analysis
> that rejects it.

Would you mind creating a commitfest entry for this one?  I'll plan on
committing this early next week unless any objections materialize.

-- 
nathan



Re: CREATE MATERIALIZED VIEW

From
Dagfinn Ilmari Mannsåker
Date:
Nathan Bossart <nathandbossart@gmail.com> writes:

> On Thu, Jul 25, 2024 at 03:49:02PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Nathan Bossart <nathandbossart@gmail.com> writes:
>>> However, since it looks like the feature was never actually supported in a
>>> release, and the revert has been in place for over a decade, I think it'd
>>> be reasonable to remove the tab completion now.  It looks like the folks on
>>> the 2021 thread felt similarly.
>> 
>> Keeping it in the grammar makes sense for the more specific error
>> message, but I don't think the tab completion should be suggesting bogus
>> things, regardless of whether it's the grammar or the parse analysis
>> that rejects it.
>
> Would you mind creating a commitfest entry for this one?  I'll plan on
> committing this early next week unless any objections materialize.

Done: https://commitfest.postgresql.org/49/5139/

I've taken the liberty of setting you as the committer, and the target
version to 17 even though it turns out to be an older bug, since it's
arguably a follow-on fix to the incomplete fix in c951e9042dd1.

- ilmari



Re: CREATE MATERIALIZED VIEW

From
Nathan Bossart
Date:
On Thu, Jul 25, 2024 at 04:10:37PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Done: https://commitfest.postgresql.org/49/5139/
> 
> I've taken the liberty of setting you as the committer, and the target
> version to 17 even though it turns out to be an older bug, since it's
> arguably a follow-on fix to the incomplete fix in c951e9042dd1.

Thanks.  I'm -1 on back-patching since it was intentionally left around and
is basically harmless.

-- 
nathan



Re: CREATE MATERIALIZED VIEW

From
Nathan Bossart
Date:
Committed.

-- 
nathan



Re: CREATE MATERIALIZED VIEW

From
Dagfinn Ilmari Mannsåker
Date:
Nathan Bossart <nathandbossart@gmail.com> writes:

> Committed.

Thanks!

- ilmari