On 2017/02/20 1:22, Robert Haas wrote: > On Thu, Feb 16, 2017 at 7:15 AM, Amit Langote > <langote_amit...@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
>From 898c4d77ceff443b3170b793a0c95a0b793c544a Mon Sep 17 00:00:00 2001 From: amit <amitlangot...@gmail.com> Date: Mon, 13 Feb 2017 18:18:48 +0900 Subject: [PATCH] Tab completion for the new partitioning syntax --- src/bin/psql/tab-complete.c | 57 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 94814c20d0..ddad71a10f 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -463,6 +463,21 @@ static const SchemaQuery Query_for_list_of_tables = { NULL }; +static const SchemaQuery Query_for_list_of_partitioned_tables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind IN ('P')", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL +}; + static const SchemaQuery Query_for_list_of_constraints_with_schema = { /* catname */ "pg_catalog.pg_constraint c", @@ -913,6 +928,16 @@ static const SchemaQuery Query_for_list_of_matviews = { " SELECT 'DEFAULT' ) ss "\ " WHERE pg_catalog.substring(name,1,%%d)='%%s'" +/* the silly-looking length condition is just to eat up the current word */ +#define Query_for_partition_of_table \ +"SELECT pg_catalog.quote_ident(c2.relname) "\ +" FROM pg_catalog.pg_class c1, pg_catalog.pg_class c2, pg_catalog.pg_inherits i"\ +" WHERE c1.oid=i.inhparent and i.inhrelid=c2.oid"\ +" and (%d = pg_catalog.length('%s'))"\ +" and pg_catalog.quote_ident(c1.relname)='%s'"\ +" and pg_catalog.pg_table_is_visible(c2.oid)"\ +" and c2.relispartition = 'true'" + /* * This is a list of all "things" in Pgsql, which can show up after CREATE or * DROP; and there is also a query to get a list of them. @@ -1742,7 +1767,8 @@ psql_completion(const char *text, int start, int end) static const char *const list_ALTER2[] = {"ADD", "ALTER", "CLUSTER ON", "DISABLE", "DROP", "ENABLE", "INHERIT", "NO INHERIT", "RENAME", "RESET", "OWNER TO", "SET", - "VALIDATE CONSTRAINT", "REPLICA IDENTITY", NULL}; + "VALIDATE CONSTRAINT", "REPLICA IDENTITY", "ATTACH PARTITION", + "DETACH PARTITION", NULL}; COMPLETE_WITH_LIST(list_ALTER2); } @@ -1923,6 +1949,26 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH_LIST4("FULL", "NOTHING", "DEFAULT", "USING"); else if (Matches4("ALTER", "TABLE", MatchAny, "REPLICA")) COMPLETE_WITH_CONST("IDENTITY"); + /* + * If we have ALTER TABLE <foo> ATTACH PARTITION, provide a list of + * tables. + */ + else if (Matches5("ALTER", "TABLE", MatchAny, "ATTACH", "PARTITION")) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, ""); + /* Limited completion support for partition bound specification */ + else if (TailMatches3("ATTACH", "PARTITION", MatchAny)) + COMPLETE_WITH_CONST("FOR VALUES"); + else if (TailMatches2("FOR", "VALUES")) + COMPLETE_WITH_LIST2("FROM (", "IN ("); + /* + * If we have ALTER TABLE <foo> DETACH PARTITION, provide a list of + * partitions of <foo>. + */ + else if (Matches5("ALTER", "TABLE", MatchAny, "DETACH", "PARTITION")) + { + completion_info_charp = prev3_wd; + COMPLETE_WITH_QUERY(Query_for_partition_of_table); + } /* ALTER TABLESPACE <foo> with RENAME TO, OWNER TO, SET, RESET */ else if (Matches3("ALTER", "TABLESPACE", MatchAny)) @@ -2300,6 +2346,15 @@ psql_completion(const char *text, int start, int end) /* Complete "CREATE UNLOGGED" with TABLE or MATVIEW */ else if (TailMatches2("CREATE", "UNLOGGED")) COMPLETE_WITH_LIST2("TABLE", "MATERIALIZED VIEW"); + /* Complete PARTITION BY with RANGE ( or LIST ( or ... */ + else if (TailMatches2("PARTITION", "BY")) + COMPLETE_WITH_LIST2("RANGE (", "LIST ("); + /* If we have xxx PARTITION OF, provide a list of partitioned tables */ + else if (TailMatches2("PARTITION", "OF")) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_partitioned_tables, ""); + /* Limited completion support for partition bound specification */ + else if (TailMatches3("PARTITION", "OF", MatchAny)) + COMPLETE_WITH_CONST("FOR VALUES"); /* CREATE TABLESPACE */ else if (Matches3("CREATE", "TABLESPACE", MatchAny)) -- 2.11.0
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers