tab completion for partitioning

Started by Robert Haasabout 9 years ago3 messageshackers
Jump to latest
#1Robert Haas
robertmhaas@gmail.com

On Thu, Feb 16, 2017 at 7:15 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Also attaching 0002 (unchanged) for tab-completion support for the new
partitioning syntax.

At one point you have this:

+    /* Limited completion support for partition bound specification */
+    else if (TailMatches3("ATTACH", "PARTITION", MatchAny))
+        COMPLETE_WITH_CONST("FOR VALUES");
+    else if (TailMatches5("ATTACH", "PARTITION", MatchAny, "FOR", "VALUES"))
+        COMPLETE_WITH_LIST2("FROM (", "IN (");
+    /*

And then later on you have it again:

+    /* Limited completion support for partition bound specification */
+    else if (TailMatches3("PARTITION", "OF", MatchAny))
+        COMPLETE_WITH_CONST("FOR VALUES");
+    else if (TailMatches5("PARTITION", "OF", MatchAny, "FOR", "VALUES"))
+        COMPLETE_WITH_LIST2("FROM (", "IN (");

I don't think there's any benefit in repeating this. I'm not sure
which location to keep, but it doesn't seem to make sense to have it
in two places.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Robert Haas (#1)
Re: tab completion for partitioning

On 2017/02/20 1:22, Robert Haas wrote:

On Thu, Feb 16, 2017 at 7:15 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Also attaching 0002 (unchanged) for tab-completion support for the new
partitioning syntax.

At one point you have this:

+    /* Limited completion support for partition bound specification */
+    else if (TailMatches3("ATTACH", "PARTITION", MatchAny))
+        COMPLETE_WITH_CONST("FOR VALUES");
+    else if (TailMatches5("ATTACH", "PARTITION", MatchAny, "FOR", "VALUES"))
+        COMPLETE_WITH_LIST2("FROM (", "IN (");
+    /*

And then later on you have it again:

+    /* Limited completion support for partition bound specification */
+    else if (TailMatches3("PARTITION", "OF", MatchAny))
+        COMPLETE_WITH_CONST("FOR VALUES");
+    else if (TailMatches5("PARTITION", "OF", MatchAny, "FOR", "VALUES"))
+        COMPLETE_WITH_LIST2("FROM (", "IN (");

I don't think there's any benefit in repeating this. I'm not sure
which location to keep, but it doesn't seem to make sense to have it
in two places.

Thanks for taking a look. Hm, I think the second part seems to be
needless duplication. So, I changed it to match using TailMatches2("FOR",
"VALUES") and kept just one instance of it. The first part is matching
and completing two different commands (ATTACH PARTITION partition_name and
PARTITION OF parent_name), so that seems fine.

Updated patch attached.

Thanks,
Amit

Attachments:

0001-Tab-completion-for-the-new-partitioning-syntax.patchtext/x-diff; name=0001-Tab-completion-for-the-new-partitioning-syntax.patchDownload+56-2
#3Robert Haas
robertmhaas@gmail.com
In reply to: Amit Langote (#2)
Re: tab completion for partitioning

On Mon, Feb 20, 2017 at 8:37 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Thanks for taking a look. Hm, I think the second part seems to be
needless duplication. So, I changed it to match using TailMatches2("FOR",
"VALUES") and kept just one instance of it. The first part is matching
and completing two different commands (ATTACH PARTITION partition_name and
PARTITION OF parent_name), so that seems fine.

I agree. Committed this version.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers