Thread: [HACKERS] tab completion for partitioning
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
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
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