Re: Converting tab-complete.c's else-if chain to a switch - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: Converting tab-complete.c's else-if chain to a switch |
Date | |
Msg-id | 1809543.1722024167@sss.pgh.pa.us Whole thread Raw |
In response to | Re: Converting tab-complete.c's else-if chain to a switch (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
I wrote: > I modified the preprocessor to work like that, and I like the results > better than what I had. I thought this version of the patch would be less subject to merge conflicts than v1, but it didn't take long for somebody to break it. Rebased v3 attached -- no nontrivial changes from v2. I'd like to get this merged soon ... regards, tom lane From 273ec2558fad646a2835bfe795fb12b8e821a12f Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 26 Jul 2024 15:41:20 -0400 Subject: [PATCH v3 1/3] Invent "MatchAnyN" option for tab-complete.c's Matches/MatchesCS. This argument matches any number (including zero) of previous words. Use it to replace the common coding pattern if (HeadMatches("A", "B") && TailMatches("X", "Y")) with if (Matches("A", "B", MatchAnyN, "X", "Y")) In itself this feature doesn't do much except (arguably) make the code slightly shorter and more readable. However, it reduces the number of complex if-condition patterns that have to be dealt with in the next commits in this series. While here, restructure the *Matches implementation functions so that the actual work is done in functions that take a char ** array of pattern strings, and the versions taking variadic arguments are thin wrappers around the array ones. This simplifies the new Matches logic considerably. At the end of this patch series, the array functions will be the only ones that are material to performance, so having the variadic ones be wrappers makes sense. Discussion: https://postgr.es/m/2208466.1720729502@sss.pgh.pa.us --- src/bin/psql/tab-complete.c | 400 +++++++++++++++++++++++------------- 1 file changed, 258 insertions(+), 142 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 891face1b6..8f68bfc54d 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1446,10 +1446,12 @@ initialize_readline(void) * * For readability, callers should use the macros MatchAny and MatchAnyExcept * to invoke those two special cases for 'pattern'. (But '|' and '*' must - * just be written directly in patterns.) + * just be written directly in patterns.) There is also MatchAnyN, but that + * is supported only in Matches/MatchesCS and is not handled here. */ #define MatchAny NULL #define MatchAnyExcept(pattern) ("!" pattern) +#define MatchAnyN "" static bool word_matches(const char *pattern, @@ -1514,107 +1516,196 @@ word_matches(const char *pattern, } /* - * Implementation of TailMatches and TailMatchesCS macros: do the last N words - * in previous_words match the variadic arguments? + * Implementation of TailMatches and TailMatchesCS tests: do the last N words + * in previous_words match the pattern arguments? * * The array indexing might look backwards, but remember that * previous_words[0] contains the *last* word on the line, not the first. */ static bool -TailMatchesImpl(bool case_sensitive, - int previous_words_count, char **previous_words, - int narg,...) +TailMatchesArray(bool case_sensitive, + int previous_words_count, char **previous_words, + int narg, const char *const *args) { - va_list args; - if (previous_words_count < narg) return false; - va_start(args, narg); - for (int argno = 0; argno < narg; argno++) { - const char *arg = va_arg(args, const char *); + const char *arg = args[argno]; if (!word_matches(arg, previous_words[narg - argno - 1], case_sensitive)) - { - va_end(args); return false; - } } - va_end(args); - return true; } /* - * Implementation of Matches and MatchesCS macros: do all of the words - * in previous_words match the variadic arguments? + * As above, but the pattern is passed as a variadic argument list. */ static bool -MatchesImpl(bool case_sensitive, - int previous_words_count, char **previous_words, - int narg,...) +TailMatchesImpl(bool case_sensitive, + int previous_words_count, char **previous_words, + int narg,...) { + const char *argarray[64]; va_list args; - if (previous_words_count != narg) + Assert(narg <= lengthof(argarray)); + + if (previous_words_count < narg) return false; va_start(args, narg); + for (int argno = 0; argno < narg; argno++) + argarray[argno] = va_arg(args, const char *); + va_end(args); + + return TailMatchesArray(case_sensitive, + previous_words_count, previous_words, + narg, argarray); +} + +/* + * Implementation of HeadMatches and HeadMatchesCS tests: do the first N + * words in previous_words match the pattern arguments? + */ +static bool +HeadMatchesArray(bool case_sensitive, + int previous_words_count, char **previous_words, + int narg, const char *const *args) +{ + if (previous_words_count < narg) + return false; for (int argno = 0; argno < narg; argno++) { - const char *arg = va_arg(args, const char *); + const char *arg = args[argno]; - if (!word_matches(arg, previous_words[narg - argno - 1], + if (!word_matches(arg, previous_words[previous_words_count - argno - 1], case_sensitive)) - { - va_end(args); return false; - } } - va_end(args); - return true; } /* - * Implementation of HeadMatches and HeadMatchesCS macros: do the first N - * words in previous_words match the variadic arguments? + * As above, but the pattern is passed as a variadic argument list. */ static bool HeadMatchesImpl(bool case_sensitive, int previous_words_count, char **previous_words, int narg,...) { + const char *argarray[64]; va_list args; + Assert(narg <= lengthof(argarray)); + if (previous_words_count < narg) return false; va_start(args, narg); + for (int argno = 0; argno < narg; argno++) + argarray[argno] = va_arg(args, const char *); + va_end(args); + return HeadMatchesArray(case_sensitive, + previous_words_count, previous_words, + narg, argarray); +} + +/* + * Implementation of Matches and MatchesCS tests: do all of the words + * in previous_words match the pattern arguments? + * + * This supports an additional kind of wildcard: MatchAnyN (represented as "") + * can match any number of words, including zero, in the middle of the list. + */ +static bool +MatchesArray(bool case_sensitive, + int previous_words_count, char **previous_words, + int narg, const char *const *args) +{ + int match_any_pos = -1; + + /* Even with MatchAnyN, there must be at least N-1 words */ + if (previous_words_count < narg - 1) + return false; + + /* Check for MatchAnyN */ for (int argno = 0; argno < narg; argno++) { - const char *arg = va_arg(args, const char *); + const char *arg = args[argno]; - if (!word_matches(arg, previous_words[previous_words_count - argno - 1], - case_sensitive)) + if (arg != NULL && arg[0] == '\0') { - va_end(args); - return false; + match_any_pos = argno; + break; } } - va_end(args); + if (match_any_pos < 0) + { + /* Standard case without MatchAnyN */ + if (previous_words_count != narg) + return false; + + /* Either Head or Tail match will do for the rest */ + if (!HeadMatchesArray(case_sensitive, + previous_words_count, previous_words, + narg, args)) + return false; + } + else + { + /* Match against head */ + if (!HeadMatchesArray(case_sensitive, + previous_words_count, previous_words, + match_any_pos, args)) + return false; + + /* Match against tail */ + if (!TailMatchesArray(case_sensitive, + previous_words_count, previous_words, + narg - match_any_pos - 1, + args + match_any_pos + 1)) + return false; + } return true; } +/* + * As above, but the pattern is passed as a variadic argument list. + */ +static bool +MatchesImpl(bool case_sensitive, + int previous_words_count, char **previous_words, + int narg,...) +{ + const char *argarray[64]; + va_list args; + + Assert(narg <= lengthof(argarray)); + + /* Even with MatchAnyN, there must be at least N-1 words */ + if (previous_words_count < narg - 1) + return false; + + va_start(args, narg); + for (int argno = 0; argno < narg; argno++) + argarray[argno] = va_arg(args, const char *); + va_end(args); + + return MatchesArray(case_sensitive, + previous_words_count, previous_words, + narg, argarray); +} + /* * Check if the final character of 's' is 'c'. */ @@ -1906,9 +1997,9 @@ psql_completion(const char *text, int start, int end) * "ALTER PUBLICATION <name> ADD TABLE <name> WHERE (" - complete with * table attributes */ - else if (HeadMatches("ALTER", "PUBLICATION", MatchAny) && TailMatches("WHERE")) + else if (Matches("ALTER", "PUBLICATION", MatchAny, MatchAnyN, "WHERE")) COMPLETE_WITH("("); - else if (HeadMatches("ALTER", "PUBLICATION", MatchAny) && TailMatches("WHERE", "(")) + else if (Matches("ALTER", "PUBLICATION", MatchAny, MatchAnyN, "WHERE", "(")) COMPLETE_WITH_ATTR(prev3_wd); else if (HeadMatches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") && !TailMatches("WHERE", "(*)")) @@ -1926,7 +2017,7 @@ psql_completion(const char *text, int start, int end) " AND nspname NOT LIKE E'pg\\\\_%%'", "CURRENT_SCHEMA"); /* ALTER PUBLICATION <name> SET ( */ - else if (HeadMatches("ALTER", "PUBLICATION", MatchAny) && TailMatches("SET", "(")) + else if (Matches("ALTER", "PUBLICATION", MatchAny, MatchAnyN, "SET", "(")) COMPLETE_WITH("publish", "publish_via_partition_root"); /* ALTER SUBSCRIPTION <name> */ else if (Matches("ALTER", "SUBSCRIPTION", MatchAny)) @@ -1934,36 +2025,34 @@ psql_completion(const char *text, int start, int end) "RENAME TO", "REFRESH PUBLICATION", "SET", "SKIP (", "ADD PUBLICATION", "DROP PUBLICATION"); /* ALTER SUBSCRIPTION <name> REFRESH PUBLICATION */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && - TailMatches("REFRESH", "PUBLICATION")) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "REFRESH", "PUBLICATION")) COMPLETE_WITH("WITH ("); /* ALTER SUBSCRIPTION <name> REFRESH PUBLICATION WITH ( */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && - TailMatches("REFRESH", "PUBLICATION", "WITH", "(")) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "REFRESH", "PUBLICATION", "WITH", "(")) COMPLETE_WITH("copy_data"); /* ALTER SUBSCRIPTION <name> SET */ else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, "SET")) COMPLETE_WITH("(", "PUBLICATION"); /* ALTER SUBSCRIPTION <name> SET ( */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "(")) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "SET", "(")) COMPLETE_WITH("binary", "disable_on_error", "failover", "origin", "password_required", "run_as_owner", "slot_name", "streaming", "synchronous_commit", "two_phase"); /* ALTER SUBSCRIPTION <name> SKIP ( */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SKIP", "(")) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "SKIP", "(")) COMPLETE_WITH("lsn"); /* ALTER SUBSCRIPTION <name> SET PUBLICATION */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "PUBLICATION")) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, "SET", "PUBLICATION")) { /* complete with nothing here as this refers to remote publications */ } /* ALTER SUBSCRIPTION <name> ADD|DROP|SET PUBLICATION <name> */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && - TailMatches("ADD|DROP|SET", "PUBLICATION", MatchAny)) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, + "ADD|DROP|SET", "PUBLICATION", MatchAny)) COMPLETE_WITH("WITH ("); /* ALTER SUBSCRIPTION <name> ADD|DROP|SET PUBLICATION <name> WITH ( */ - else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && - TailMatches("ADD|DROP|SET", "PUBLICATION", MatchAny, "WITH", "(")) + else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, MatchAnyN, + "ADD|DROP|SET", "PUBLICATION", MatchAny, "WITH", "(")) COMPLETE_WITH("copy_data", "refresh"); /* ALTER SCHEMA <name> */ @@ -2726,7 +2815,7 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("VERBOSE|SKIP_LOCKED")) COMPLETE_WITH("ON", "OFF"); } - else if (HeadMatches("ANALYZE") && TailMatches("(")) + else if (Matches("ANALYZE", MatchAnyN, "(")) /* "ANALYZE (" should be caught above, so assume we want columns */ COMPLETE_WITH_ATTR(prev2_wd); else if (HeadMatches("ANALYZE")) @@ -3178,11 +3267,11 @@ psql_completion(const char *text, int start, int end) * "CREATE PUBLICATION <name> FOR TABLE <name> WHERE (" - complete with * table attributes */ - else if (HeadMatches("CREATE", "PUBLICATION", MatchAny) && TailMatches("WHERE")) + else if (Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WHERE")) COMPLETE_WITH("("); - else if (HeadMatches("CREATE", "PUBLICATION", MatchAny) && TailMatches("WHERE", "(")) + else if (Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WHERE", "(")) COMPLETE_WITH_ATTR(prev3_wd); - else if (HeadMatches("CREATE", "PUBLICATION", MatchAny) && TailMatches("WHERE", "(*)")) + else if (Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WHERE", "(*)")) COMPLETE_WITH(" WITH ("); /* @@ -3195,7 +3284,7 @@ psql_completion(const char *text, int start, int end) else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES", "IN", "SCHEMA", MatchAny) && (!ends_with(prev_wd,','))) COMPLETE_WITH("WITH ("); /* Complete "CREATE PUBLICATION <name> [...] WITH" */ - else if (HeadMatches("CREATE", "PUBLICATION") && TailMatches("WITH", "(")) + else if (Matches("CREATE", "PUBLICATION", MatchAnyN, "WITH", "(")) COMPLETE_WITH("publish", "publish_via_partition_root"); /* CREATE RULE */ @@ -3259,8 +3348,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("ndistinct", "dependencies", "mcv"); else if (Matches("CREATE", "STATISTICS", MatchAny, "(*)")) COMPLETE_WITH("ON"); - else if (HeadMatches("CREATE", "STATISTICS", MatchAny) && - TailMatches("FROM")) + else if (Matches("CREATE", "STATISTICS", MatchAny, MatchAnyN, "FROM")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); /* CREATE TABLE --- is allowed inside CREATE SCHEMA, so use TailMatches */ @@ -3358,10 +3446,10 @@ psql_completion(const char *text, int start, int end) { /* complete with nothing here as this refers to remote publications */ } - else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("PUBLICATION", MatchAny)) + else if (Matches("CREATE", "SUBSCRIPTION", MatchAnyN, "PUBLICATION", MatchAny)) COMPLETE_WITH("WITH ("); /* Complete "CREATE SUBSCRIPTION <name> ... WITH ( <opt>" */ - else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH", "(")) + else if (Matches("CREATE", "SUBSCRIPTION", MatchAnyN, "WITH", "(")) COMPLETE_WITH("binary", "connect", "copy_data", "create_slot", "disable_on_error", "enabled", "failover", "origin", "password_required", "run_as_owner", "slot_name", @@ -3415,9 +3503,10 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny, "ON") || TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny, "ON")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_views); - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("ON", MatchAny)) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "ON", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "ON", MatchAny)) { if (pset.sversion >= 110000) COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY", @@ -3426,76 +3515,108 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY", "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - (TailMatches("DEFERRABLE") || TailMatches("INITIALLY", "IMMEDIATE|DEFERRED"))) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "DEFERRABLE") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "DEFERRABLE") || + Matches("CREATE", "TRIGGER", MatchAnyN, + "INITIALLY", "IMMEDIATE|DEFERRED") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "INITIALLY", "IMMEDIATE|DEFERRED")) { if (pset.sversion >= 110000) COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION"); else COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("REFERENCING")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING")) COMPLETE_WITH("OLD TABLE", "NEW TABLE"); - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("OLD|NEW", "TABLE")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "OLD|NEW", "TABLE") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "OLD|NEW", "TABLE")) COMPLETE_WITH("AS"); - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - (TailMatches("REFERENCING", "OLD", "TABLE", "AS", MatchAny) || - TailMatches("REFERENCING", "OLD", "TABLE", MatchAny))) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD", "TABLE", "AS", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD", "TABLE", "AS", MatchAny) || + Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD", "TABLE", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD", "TABLE", MatchAny)) { if (pset.sversion >= 110000) COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE FUNCTION"); else COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - (TailMatches("REFERENCING", "NEW", "TABLE", "AS", MatchAny) || - TailMatches("REFERENCING", "NEW", "TABLE", MatchAny))) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "NEW", "TABLE", "AS", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "NEW", "TABLE", "AS", MatchAny) || + Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "NEW", "TABLE", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "NEW", "TABLE", MatchAny)) { if (pset.sversion >= 110000) COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE FUNCTION"); else COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - (TailMatches("REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) || - TailMatches("REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) || - TailMatches("REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", MatchAny) || - TailMatches("REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", MatchAny))) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) || + Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) || + Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", MatchAny) || + Matches("CREATE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", MatchAny) || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", MatchAny)) { if (pset.sversion >= 110000) COMPLETE_WITH("FOR", "WHEN (", "EXECUTE FUNCTION"); else COMPLETE_WITH("FOR", "WHEN (", "EXECUTE PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("FOR")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "FOR") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "FOR")) COMPLETE_WITH("EACH", "ROW", "STATEMENT"); - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("FOR", "EACH")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "FOR", "EACH") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "FOR", "EACH")) COMPLETE_WITH("ROW", "STATEMENT"); - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - (TailMatches("FOR", "EACH", "ROW|STATEMENT") || - TailMatches("FOR", "ROW|STATEMENT"))) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "FOR", "EACH", "ROW|STATEMENT") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "FOR", "EACH", "ROW|STATEMENT") || + Matches("CREATE", "TRIGGER", MatchAnyN, + "FOR", "ROW|STATEMENT") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "FOR", "ROW|STATEMENT")) { if (pset.sversion >= 110000) COMPLETE_WITH("WHEN (", "EXECUTE FUNCTION"); else COMPLETE_WITH("WHEN (", "EXECUTE PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("WHEN", "(*)")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "WHEN", "(*)") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "WHEN", "(*)")) { if (pset.sversion >= 110000) COMPLETE_WITH("EXECUTE FUNCTION"); @@ -3507,18 +3628,20 @@ psql_completion(const char *text, int start, int end) * Complete CREATE [ OR REPLACE ] TRIGGER ... EXECUTE with * PROCEDURE|FUNCTION. */ - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("EXECUTE")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "EXECUTE") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "EXECUTE")) { if (pset.sversion >= 110000) COMPLETE_WITH("FUNCTION"); else COMPLETE_WITH("PROCEDURE"); } - else if ((HeadMatches("CREATE", "TRIGGER") || - HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && - TailMatches("EXECUTE", "FUNCTION|PROCEDURE")) + else if (Matches("CREATE", "TRIGGER", MatchAnyN, + "EXECUTE", "FUNCTION|PROCEDURE") || + Matches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAnyN, + "EXECUTE", "FUNCTION|PROCEDURE")) COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_functions); /* CREATE ROLE,USER,GROUP <name> */ @@ -3651,16 +3774,14 @@ psql_completion(const char *text, int start, int end) else COMPLETE_WITH("WHEN TAG IN (", "EXECUTE PROCEDURE"); } - else if (HeadMatches("CREATE", "EVENT", "TRIGGER") && - TailMatches("WHEN|AND", MatchAny, "IN", "(*)")) + else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAnyN, "WHEN|AND", MatchAny, "IN", "(*)")) { if (pset.sversion >= 110000) COMPLETE_WITH("EXECUTE FUNCTION"); else COMPLETE_WITH("EXECUTE PROCEDURE"); } - else if (HeadMatches("CREATE", "EVENT", "TRIGGER") && - TailMatches("EXECUTE", "FUNCTION|PROCEDURE")) + else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAnyN, "EXECUTE", "FUNCTION|PROCEDURE")) COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_functions); /* DEALLOCATE */ @@ -3685,27 +3806,27 @@ psql_completion(const char *text, int start, int end) * provides, like the syntax of DECLARE command in the documentation * indicates. */ - else if (HeadMatches("DECLARE") && TailMatches("BINARY")) + else if (Matches("DECLARE", MatchAnyN, "BINARY")) COMPLETE_WITH("ASENSITIVE", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR"); - else if (HeadMatches("DECLARE") && TailMatches("ASENSITIVE|INSENSITIVE")) + else if (Matches("DECLARE", MatchAnyN, "ASENSITIVE|INSENSITIVE")) COMPLETE_WITH("SCROLL", "NO SCROLL", "CURSOR"); - else if (HeadMatches("DECLARE") && TailMatches("SCROLL")) + else if (Matches("DECLARE", MatchAnyN, "SCROLL")) COMPLETE_WITH("CURSOR"); /* Complete DECLARE ... [options] NO with SCROLL */ - else if (HeadMatches("DECLARE") && TailMatches("NO")) + else if (Matches("DECLARE", MatchAnyN, "NO")) COMPLETE_WITH("SCROLL"); /* * Complete DECLARE ... CURSOR with one of WITH HOLD, WITHOUT HOLD, and * FOR */ - else if (HeadMatches("DECLARE") && TailMatches("CURSOR")) + else if (Matches("DECLARE", MatchAnyN, "CURSOR")) COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR"); /* Complete DECLARE ... CURSOR WITH|WITHOUT with HOLD */ - else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT")) + else if (Matches("DECLARE", MatchAnyN, "CURSOR", "WITH|WITHOUT")) COMPLETE_WITH("HOLD"); /* Complete DECLARE ... CURSOR WITH|WITHOUT HOLD with FOR */ - else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT", "HOLD")) + else if (Matches("DECLARE", MatchAnyN, "CURSOR", "WITH|WITHOUT", "HOLD")) COMPLETE_WITH("FOR"); /* DELETE --- can be inside EXPLAIN, RULE, etc */ @@ -3930,8 +4051,7 @@ psql_completion(const char *text, int start, int end) "FROM", "IN"); /* Complete FETCH <direction> "FROM" or "IN" with a list of cursors */ - else if (HeadMatches("FETCH|MOVE") && - TailMatches("FROM|IN")) + else if (Matches("FETCH|MOVE", MatchAnyN, "FROM|IN")) COMPLETE_WITH_QUERY(Query_for_list_of_cursors); /* FOREIGN DATA WRAPPER */ @@ -3940,8 +4060,7 @@ psql_completion(const char *text, int start, int end) !TailMatches("CREATE", MatchAny, MatchAny, MatchAny)) COMPLETE_WITH_QUERY(Query_for_list_of_fdws); /* applies in CREATE SERVER */ - else if (TailMatches("FOREIGN", "DATA", "WRAPPER", MatchAny) && - HeadMatches("CREATE", "SERVER")) + else if (Matches("CREATE", "SERVER", MatchAnyN, "FOREIGN", "DATA", "WRAPPER", MatchAny)) COMPLETE_WITH("OPTIONS"); /* FOREIGN TABLE */ @@ -4128,44 +4247,43 @@ psql_completion(const char *text, int start, int end) * Complete "GRANT/REVOKE ... TO/FROM" with username, PUBLIC, * CURRENT_ROLE, CURRENT_USER, or SESSION_USER. */ - else if ((HeadMatches("GRANT") && TailMatches("TO")) || - (HeadMatches("REVOKE") && TailMatches("FROM"))) + else if (Matches("GRANT", MatchAnyN, "TO") || + Matches("REVOKE", MatchAnyN, "FROM")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, Keywords_for_list_of_grant_roles); /* * Offer grant options after that. */ - else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny)) + else if (Matches("GRANT", MatchAnyN, "TO", MatchAny)) COMPLETE_WITH("WITH ADMIN", "WITH INHERIT", "WITH SET", "WITH GRANT OPTION", "GRANTED BY"); - else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH")) + else if (Matches("GRANT", MatchAnyN, "TO", MatchAny, "WITH")) COMPLETE_WITH("ADMIN", "INHERIT", "SET", "GRANT OPTION"); - else if (HeadMatches("GRANT") && - (TailMatches("TO", MatchAny, "WITH", "ADMIN|INHERIT|SET"))) + else if (Matches("GRANT", MatchAnyN, "TO", MatchAny, "WITH", "ADMIN|INHERIT|SET")) COMPLETE_WITH("OPTION", "TRUE", "FALSE"); - else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", MatchAny, "OPTION")) + else if (Matches("GRANT", MatchAnyN, "TO", MatchAny, "WITH", MatchAny, "OPTION")) COMPLETE_WITH("GRANTED BY"); - else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", MatchAny, "OPTION", "GRANTED", "BY")) + else if (Matches("GRANT", MatchAnyN, "TO", MatchAny, "WITH", MatchAny, "OPTION", "GRANTED", "BY")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, Keywords_for_list_of_grant_roles); /* Complete "ALTER DEFAULT PRIVILEGES ... GRANT/REVOKE ... TO/FROM */ - else if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES") && TailMatches("TO|FROM")) + else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", MatchAnyN, "TO|FROM")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, Keywords_for_list_of_grant_roles); /* Offer WITH GRANT OPTION after that */ - else if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES") && TailMatches("TO", MatchAny)) + else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", MatchAnyN, "TO", MatchAny)) COMPLETE_WITH("WITH GRANT OPTION"); /* Complete "GRANT/REVOKE ... ON * *" with TO/FROM */ - else if (HeadMatches("GRANT") && TailMatches("ON", MatchAny, MatchAny)) + else if (Matches("GRANT", MatchAnyN, "ON", MatchAny, MatchAny)) COMPLETE_WITH("TO"); - else if (HeadMatches("REVOKE") && TailMatches("ON", MatchAny, MatchAny)) + else if (Matches("REVOKE", MatchAnyN, "ON", MatchAny, MatchAny)) COMPLETE_WITH("FROM"); /* Complete "GRANT/REVOKE * ON ALL * IN SCHEMA *" with TO/FROM */ @@ -4280,7 +4398,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("IN", "NOWAIT"); /* Complete LOCK [TABLE] [ONLY] <table> IN with a lock mode */ - else if (HeadMatches("LOCK") && TailMatches("IN")) + else if (Matches("LOCK", MatchAnyN, "IN")) COMPLETE_WITH("ACCESS SHARE MODE", "ROW SHARE MODE", "ROW EXCLUSIVE MODE", "SHARE UPDATE EXCLUSIVE MODE", "SHARE MODE", @@ -4291,16 +4409,16 @@ psql_completion(const char *text, int start, int end) * Complete LOCK [TABLE][ONLY] <table> IN ACCESS|ROW with rest of lock * mode */ - else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW")) + else if (Matches("LOCK", MatchAnyN, "IN", "ACCESS|ROW")) COMPLETE_WITH("EXCLUSIVE MODE", "SHARE MODE"); /* Complete LOCK [TABLE] [ONLY] <table> IN SHARE with rest of lock mode */ - else if (HeadMatches("LOCK") && TailMatches("IN", "SHARE")) + else if (Matches("LOCK", MatchAnyN, "IN", "SHARE")) COMPLETE_WITH("MODE", "ROW EXCLUSIVE MODE", "UPDATE EXCLUSIVE MODE"); /* Complete LOCK [TABLE] [ONLY] <table> [IN lockmode MODE] with "NOWAIT" */ - else if (HeadMatches("LOCK") && TailMatches("MODE")) + else if (Matches("LOCK", MatchAnyN, "MODE")) COMPLETE_WITH("NOWAIT"); /* MERGE --- can be inside EXPLAIN */ @@ -4605,9 +4723,7 @@ psql_completion(const char *text, int start, int end) * Complete ALTER DATABASE|FUNCTION|PROCEDURE|ROLE|ROUTINE|USER ... SET * <name> */ - else if (HeadMatches("ALTER", "DATABASE|FUNCTION|PROCEDURE|ROLE|ROUTINE|USER") && - TailMatches("SET", MatchAny) && - !TailMatches("SCHEMA")) + else if (Matches("ALTER", "DATABASE|FUNCTION|PROCEDURE|ROLE|ROUTINE|USER", MatchAnyN, "SET", MatchAnyExcept("SCHEMA"))) COMPLETE_WITH("FROM CURRENT", "TO"); /* @@ -4683,13 +4799,13 @@ psql_completion(const char *text, int start, int end) else if (Matches("TRUNCATE", "TABLE")) COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_truncatables, "ONLY"); - else if (HeadMatches("TRUNCATE") && TailMatches("ONLY")) + else if (Matches("TRUNCATE", MatchAnyN, "ONLY")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_truncatables); else if (Matches("TRUNCATE", MatchAny) || Matches("TRUNCATE", "TABLE|ONLY", MatchAny) || Matches("TRUNCATE", "TABLE", "ONLY", MatchAny)) COMPLETE_WITH("RESTART IDENTITY", "CONTINUE IDENTITY", "CASCADE", "RESTRICT"); - else if (HeadMatches("TRUNCATE") && TailMatches("IDENTITY")) + else if (Matches("TRUNCATE", MatchAnyN, "IDENTITY")) COMPLETE_WITH("CASCADE", "RESTRICT"); /* UNLISTEN */ @@ -4770,7 +4886,7 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("INDEX_CLEANUP")) COMPLETE_WITH("AUTO", "ON", "OFF"); } - else if (HeadMatches("VACUUM") && TailMatches("(")) + else if (Matches("VACUUM", MatchAnyN, "(")) /* "VACUUM (" should be caught above, so assume we want columns */ COMPLETE_WITH_ATTR(prev2_wd); else if (HeadMatches("VACUUM")) -- 2.43.5 From 90c2f5b3b126352592a3a33c31cceb3526436246 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 26 Jul 2024 15:47:54 -0400 Subject: [PATCH v3 2/3] Prepare tab-complete.c for preprocessing. Separate out psql_completion's giant else-if chain of *Matches tests into a new function. Add the infrastructure needed for table-driven checking of the initial match of each completion rule. As-is, however, the code continues to operate as it did. The new behavior applies only if SWITCH_CONVERSION_APPLIED is #defined, which it is not here. (The preprocessor added in the next patch will add a #define for that.) The first and last couple of bits of psql_completion are not based on HeadMatches/TailMatches/Matches tests, so they stay where they are; they won't become part of the switch. This patch also fixes up a couple of if-conditions that didn't meet the conditions enumerated in the comment for match_previous_words(). Those restrictions exist to simplify the preprocessor. Discussion: https://postgr.es/m/2208466.1720729502@sss.pgh.pa.us --- src/bin/psql/tab-complete.c | 344 ++++++++++++++++++++++++------- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 269 insertions(+), 76 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 8f68bfc54d..cd7640cfec 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -4,6 +4,13 @@ * Copyright (c) 2000-2024, PostgreSQL Global Development Group * * src/bin/psql/tab-complete.c + * + * Note: this will compile and work as-is if SWITCH_CONVERSION_APPLIED + * is not defined. However, the expected usage is that it's first run + * through gen_tabcomplete.pl, which will #define that symbol, fill in the + * tcpatterns[] array, and convert the else-if chain in match_previous_words() + * into a switch. See comments for match_previous_words() and the header + * comment in gen_tabcomplete.pl for more detail. */ /*---------------------------------------------------------------------- @@ -1195,6 +1202,20 @@ static const VersionedQuery Query_for_list_of_subscriptions[] = { {0, NULL} }; + /* Known command-starting keywords. */ +static const char *const sql_commands[] = { + "ABORT", "ALTER", "ANALYZE", "BEGIN", "CALL", "CHECKPOINT", "CLOSE", "CLUSTER", + "COMMENT", "COMMIT", "COPY", "CREATE", "DEALLOCATE", "DECLARE", + "DELETE FROM", "DISCARD", "DO", "DROP", "END", "EXECUTE", "EXPLAIN", + "FETCH", "GRANT", "IMPORT FOREIGN SCHEMA", "INSERT INTO", "LISTEN", "LOAD", "LOCK", + "MERGE INTO", "MOVE", "NOTIFY", "PREPARE", + "REASSIGN", "REFRESH MATERIALIZED VIEW", "REINDEX", "RELEASE", + "RESET", "REVOKE", "ROLLBACK", + "SAVEPOINT", "SECURITY LABEL", "SELECT", "SET", "SHOW", "START", + "TABLE", "TRUNCATE", "UNLISTEN", "UPDATE", "VACUUM", "VALUES", "WITH", + NULL +}; + /* * 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. @@ -1287,6 +1308,51 @@ static const pgsql_thing_t words_after_create[] = { {NULL} /* end of list */ }; +/* + * The tcpatterns[] table provides the initial pattern-match rule for each + * switch case in match_previous_words(). The contents of the table + * are constructed by gen_tabcomplete.pl. + */ + +/* Basic match rules appearing in tcpatterns[].kind */ +enum TCPatternKind +{ + Match, + MatchCS, + HeadMatch, + HeadMatchCS, + TailMatch, + TailMatchCS, +}; + +/* Things besides string literals that can appear in tcpatterns[].words */ +#define MatchAny NULL +#define MatchAnyExcept(pattern) ("!" pattern) +#define MatchAnyN "" + +/* One entry in tcpatterns[] */ +typedef struct +{ + int id; /* case label used in match_previous_words */ + enum TCPatternKind kind; /* match kind, see above */ + int nwords; /* length of words[] array */ + const char *const *words; /* array of match words */ +} TCPattern; + +/* Macro emitted by gen_tabcomplete.pl to fill a tcpatterns[] entry */ +#define TCPAT(id, kind, ...) \ + { (id), (kind), VA_ARGS_NARGS(__VA_ARGS__), \ + (const char * const []) { __VA_ARGS__ } } + +#ifdef SWITCH_CONVERSION_APPLIED + +static const TCPattern tcpatterns[] = +{ + /* Insert tab-completion pattern data here. */ +}; + +#endif /* SWITCH_CONVERSION_APPLIED */ + /* Storage parameters for CREATE TABLE and ALTER TABLE */ static const char *const table_storage_parameters[] = { "autovacuum_analyze_scale_factor", @@ -1340,6 +1406,10 @@ static const char *const view_optional_parameters[] = { /* Forward declaration of functions */ static char **psql_completion(const char *text, int start, int end); +static char **match_previous_words(int pattern_id, + const char *text, int start, int end, + char **previous_words, + int previous_words_count); static char *create_command_generator(const char *text, int state); static char *drop_command_generator(const char *text, int state); static char *alter_command_generator(const char *text, int state); @@ -1449,10 +1519,6 @@ initialize_readline(void) * just be written directly in patterns.) There is also MatchAnyN, but that * is supported only in Matches/MatchesCS and is not handled here. */ -#define MatchAny NULL -#define MatchAnyExcept(pattern) ("!" pattern) -#define MatchAnyN "" - static bool word_matches(const char *pattern, const char *word, @@ -1787,20 +1853,6 @@ psql_completion(const char *text, int start, int end) HeadMatchesImpl(true, previous_words_count, previous_words, \ VA_ARGS_NARGS(__VA_ARGS__), __VA_ARGS__) - /* Known command-starting keywords. */ - static const char *const sql_commands[] = { - "ABORT", "ALTER", "ANALYZE", "BEGIN", "CALL", "CHECKPOINT", "CLOSE", "CLUSTER", - "COMMENT", "COMMIT", "COPY", "CREATE", "DEALLOCATE", "DECLARE", - "DELETE FROM", "DISCARD", "DO", "DROP", "END", "EXECUTE", "EXPLAIN", - "FETCH", "GRANT", "IMPORT FOREIGN SCHEMA", "INSERT INTO", "LISTEN", "LOAD", "LOCK", - "MERGE INTO", "MOVE", "NOTIFY", "PREPARE", - "REASSIGN", "REFRESH MATERIALIZED VIEW", "REINDEX", "RELEASE", - "RESET", "REVOKE", "ROLLBACK", - "SAVEPOINT", "SECURITY LABEL", "SELECT", "SET", "SHOW", "START", - "TABLE", "TRUNCATE", "UNLISTEN", "UPDATE", "VACUUM", "VALUES", "WITH", - NULL - }; - /* psql's backslash commands. */ static const char *const backslash_commands[] = { "\\a", @@ -1887,6 +1939,194 @@ psql_completion(const char *text, int start, int end) else if (previous_words_count == 0) COMPLETE_WITH_LIST(sql_commands); + /* Else try completions based on matching patterns of previous words */ + else + { +#ifdef SWITCH_CONVERSION_APPLIED + /* + * If we have transformed match_previous_words into a switch, iterate + * through tcpatterns[] to see which pattern ids match. + * + * For now, we have to try the patterns in the order they are stored + * (matching the order of switch cases in match_previous_words), + * because some of the logic in match_previous_words assumes that + * previous matches have been eliminated. This is fairly + * unprincipled, and it is likely that there are undesirable as well + * as desirable interactions hidden in the order of the pattern + * checks. TODO: think about a better way to manage that. + */ + for (int tindx = 0; tindx < lengthof(tcpatterns); tindx++) + { + const TCPattern *tcpat = tcpatterns + tindx; + bool match = false; + + switch (tcpat->kind) + { + case Match: + match = MatchesArray(false, + previous_words_count, + previous_words, + tcpat->nwords, tcpat->words); + break; + case MatchCS: + match = MatchesArray(true, + previous_words_count, + previous_words, + tcpat->nwords, tcpat->words); + break; + case HeadMatch: + match = HeadMatchesArray(false, + previous_words_count, + previous_words, + tcpat->nwords, tcpat->words); + break; + case HeadMatchCS: + match = HeadMatchesArray(true, + previous_words_count, + previous_words, + tcpat->nwords, tcpat->words); + break; + case TailMatch: + match = TailMatchesArray(false, + previous_words_count, + previous_words, + tcpat->nwords, tcpat->words); + break; + case TailMatchCS: + match = TailMatchesArray(true, + previous_words_count, + previous_words, + tcpat->nwords, tcpat->words); + break; + } + if (match) + { + matches = match_previous_words(tcpat->id, text, start, end, + previous_words, + previous_words_count); + if (matches != NULL) + break; + } + } +#else /* !SWITCH_CONVERSION_APPLIED */ + /* + * If gen_tabcomplete.pl hasn't been applied to this code, just let + * match_previous_words scan through all its patterns. + */ + matches = match_previous_words(0, text, start, end, + previous_words, + previous_words_count); +#endif /* SWITCH_CONVERSION_APPLIED */ + } + + /* + * Finally, we look through the list of "things", such as TABLE, INDEX and + * check if that was the previous word. If so, execute the query to get a + * list of them. + */ + if (matches == NULL) + { + const pgsql_thing_t *wac; + + for (wac = words_after_create; wac->name != NULL; wac++) + { + if (pg_strcasecmp(prev_wd, wac->name) == 0) + { + if (wac->query) + COMPLETE_WITH_QUERY_LIST(wac->query, + wac->keywords); + else if (wac->vquery) + COMPLETE_WITH_VERSIONED_QUERY_LIST(wac->vquery, + wac->keywords); + else if (wac->squery) + COMPLETE_WITH_VERSIONED_SCHEMA_QUERY_LIST(wac->squery, + wac->keywords); + break; + } + } + } + + /* + * If we still don't have anything to match we have to fabricate some sort + * of default list. If we were to just return NULL, readline automatically + * attempts filename completion, and that's usually no good. + */ + if (matches == NULL) + { + COMPLETE_WITH_CONST(true, ""); + /* Also, prevent Readline from appending stuff to the non-match */ + rl_completion_append_character = '\0'; +#ifdef HAVE_RL_COMPLETION_SUPPRESS_QUOTE + rl_completion_suppress_quote = 1; +#endif + } + + /* free storage */ + free(previous_words); + free(words_buffer); + free(text_copy); + free(completion_ref_object); + completion_ref_object = NULL; + free(completion_ref_schema); + completion_ref_schema = NULL; + + /* Return our Grand List O' Matches */ + return matches; +} + +/* + * Subroutine to try matches based on previous_words. + * + * This can operate in one of two modes. As presented, the body of the + * function is a long if-else-if chain that sequentially tries each known + * match rule. That works, but some C compilers have trouble with such a long + * else-if chain, either taking extra time to compile or failing altogether. + * Therefore, we prefer to transform the else-if chain into a switch, and then + * each call of this function considers just one match rule (under control of + * a loop in psql_completion()). Compilers tend to be more ready to deal + * with many-arm switches than many-arm else-if chains. + * + * Each if-condition in this function must begin with a call of one of the + * functions Matches, HeadMatches, TailMatches, MatchesCS, HeadMatchesCS, or + * TailMatchesCS. The preprocessor gen_tabcomplete.pl strips out those + * calls and converts them into entries in tcpatterns[], which are evaluated + * by the calling loop in psql_completion(). Successful matches result in + * calls to this function with the appropriate pattern_id, causing just the + * corresponding switch case to be executed. + * + * If-conditions in this function can be more complex than a single *Matches + * function call in one of two ways (but not both!). They can be OR's + * of *Matches calls, such as + * else if (Matches("ALTER", "VIEW", MatchAny, "ALTER", MatchAny) || + * Matches("ALTER", "VIEW", MatchAny, "ALTER", "COLUMN", MatchAny)) + * or they can be a *Matches call AND'ed with some other condition, e.g. + * else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLE", MatchAny) && + * !ends_with(prev_wd, ',')) + * The former case is transformed into multiple tcpatterns[] entries and + * multiple case labels for the same bit of code. The latter case is + * transformed into a case label and a contained if-statement. + * + * This is split out of psql_completion() primarily to separate code that + * gen_tabcomplete.pl should process from code that it should not, although + * doing so also helps to avoid extra indentation of this code. + * + * Returns a matches list, or NULL if no match. + */ +static char ** +match_previous_words(int pattern_id, + const char *text, int start, int end, + char **previous_words, int previous_words_count) +{ + /* This is the variable we'll return. */ + char **matches = NULL; + + /* Dummy statement, allowing all the match rules to look like "else if" */ + if (0) + /* skip */ ; + + /* gen_tabcomplete.pl begins special processing here */ + /* BEGIN GEN_TABCOMPLETE */ + /* CREATE */ /* complete with something you can create */ else if (TailMatches("CREATE")) @@ -1985,9 +2225,10 @@ psql_completion(const char *text, int start, int end) /* ALTER PUBLICATION <name> ADD */ else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD")) COMPLETE_WITH("TABLES IN SCHEMA", "TABLE"); - else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") || - (HeadMatches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") && - ends_with(prev_wd, ','))) + else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE")) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); + else if (HeadMatches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") && + ends_with(prev_wd, ',')) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); /* @@ -2453,8 +2694,7 @@ psql_completion(const char *text, int start, int end) } /* ALTER TABLE xxx ADD [COLUMN] yyy */ else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN", MatchAny) || - (Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAny) && - !Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN|CONSTRAINT|CHECK|UNIQUE|PRIMARY|EXCLUDE|FOREIGN"))) + Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAnyExcept("COLUMN|CONSTRAINT|CHECK|UNIQUE|PRIMARY|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)) @@ -3855,13 +4095,14 @@ psql_completion(const char *text, int start, int end) "COLLATION|CONVERSION|DOMAIN|EXTENSION|LANGUAGE|PUBLICATION|SCHEMA|SEQUENCE|SERVER|SUBSCRIPTION|STATISTICS|TABLE|TYPE|VIEW", MatchAny) || Matches("DROP", "ACCESS", "METHOD", MatchAny) || - (Matches("DROP", "AGGREGATE|FUNCTION|PROCEDURE|ROUTINE", MatchAny, MatchAny) && - ends_with(prev_wd, ')')) || Matches("DROP", "EVENT", "TRIGGER", MatchAny) || Matches("DROP", "FOREIGN", "DATA", "WRAPPER", MatchAny) || Matches("DROP", "FOREIGN", "TABLE", MatchAny) || Matches("DROP", "TEXT", "SEARCH", "CONFIGURATION|DICTIONARY|PARSER|TEMPLATE", MatchAny)) COMPLETE_WITH("CASCADE", "RESTRICT"); + else if (Matches("DROP", "AGGREGATE|FUNCTION|PROCEDURE|ROUTINE", MatchAny, MatchAny) && + ends_with(prev_wd, ')')) + COMPLETE_WITH("CASCADE", "RESTRICT"); /* help completing some of the variants */ else if (Matches("DROP", "AGGREGATE|FUNCTION|PROCEDURE|ROUTINE", MatchAny)) @@ -5140,58 +5381,9 @@ psql_completion(const char *text, int start, int end) matches = rl_completion_matches(text, complete_from_files); } - /* - * Finally, we look through the list of "things", such as TABLE, INDEX and - * check if that was the previous word. If so, execute the query to get a - * list of them. - */ - else - { - const pgsql_thing_t *wac; - - for (wac = words_after_create; wac->name != NULL; wac++) - { - if (pg_strcasecmp(prev_wd, wac->name) == 0) - { - if (wac->query) - COMPLETE_WITH_QUERY_LIST(wac->query, - wac->keywords); - else if (wac->vquery) - COMPLETE_WITH_VERSIONED_QUERY_LIST(wac->vquery, - wac->keywords); - else if (wac->squery) - COMPLETE_WITH_VERSIONED_SCHEMA_QUERY_LIST(wac->squery, - wac->keywords); - break; - } - } - } - - /* - * If we still don't have anything to match we have to fabricate some sort - * of default list. If we were to just return NULL, readline automatically - * attempts filename completion, and that's usually no good. - */ - if (matches == NULL) - { - COMPLETE_WITH_CONST(true, ""); - /* Also, prevent Readline from appending stuff to the non-match */ - rl_completion_append_character = '\0'; -#ifdef HAVE_RL_COMPLETION_SUPPRESS_QUOTE - rl_completion_suppress_quote = 1; -#endif - } - - /* free storage */ - free(previous_words); - free(words_buffer); - free(text_copy); - free(completion_ref_object); - completion_ref_object = NULL; - free(completion_ref_schema); - completion_ref_schema = NULL; + /* gen_tabcomplete.pl ends special processing here */ + /* END GEN_TABCOMPLETE */ - /* Return our Grand List O' Matches */ return matches; } diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b4d7f9217c..ca5c476cca 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2803,6 +2803,7 @@ TBMSharedIterator TBMSharedIteratorState TBMStatus TBlockState +TCPattern TIDBitmap TM_FailureData TM_IndexDelete -- 2.43.5 From 7d58f2d9d267830dfb0768d7ac2aeabb0669a219 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 26 Jul 2024 15:53:52 -0400 Subject: [PATCH v3 3/3] Convert tab-complete's long else-if chain to a switch statement. Rename tab-complete.c to tab-complete.in.c, create the preprocessor script gen_tabcomplete.pl, and install Makefile/meson.build rules to create tab-complete.c from tab-complete.in.c. The preprocessor converts match_previous_words' else-if chain into a switch and populates tcpatterns[] with the data needed by the driver loop. The initial HeadMatches/TailMatches/Matches test in each else-if arm is now performed in a table-driven loop. Where we get a match, the corresponding switch case is invoked to see if the match succeeds. (It might not, if there were additional conditions in the original else-if test.) The total number of string comparisons done is just about the same as it was in the previous coding; however, now that we have table-driven logic underlying the handmade rules, there is room to improve that. For now I haven't bothered because tab completion is still plenty fast enough for human use. If the number of rules keeps increasing, we might someday need to do more in that area. The immediate benefit of all this thrashing is that C compilers frequently don't deal well with long else-if chains. On gcc 8.5.0, this reduces the compile time of tab-complete.c by about a factor of four, while MSVC is reported to crash outright with the previous coding. Discussion: https://postgr.es/m/2208466.1720729502@sss.pgh.pa.us --- src/bin/psql/.gitignore | 1 + src/bin/psql/Makefile | 5 +- src/bin/psql/gen_tabcomplete.pl | 306 ++++++++++++++++++ src/bin/psql/meson.build | 12 +- .../{tab-complete.c => tab-complete.in.c} | 2 +- 5 files changed, 323 insertions(+), 3 deletions(-) create mode 100644 src/bin/psql/gen_tabcomplete.pl rename src/bin/psql/{tab-complete.c => tab-complete.in.c} (99%) diff --git a/src/bin/psql/.gitignore b/src/bin/psql/.gitignore index 10b6dd3a6b..7272f6e35d 100644 --- a/src/bin/psql/.gitignore +++ b/src/bin/psql/.gitignore @@ -1,4 +1,5 @@ /psqlscanslash.c +/tab-complete.c /sql_help.h /sql_help.c /psql diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile index 374c4c3ab8..62636d2663 100644 --- a/src/bin/psql/Makefile +++ b/src/bin/psql/Makefile @@ -62,6 +62,9 @@ psqlscanslash.c: FLEXFLAGS = -Cfe -p -p psqlscanslash.c: FLEX_NO_BACKUP=yes psqlscanslash.c: FLEX_FIX_WARNING=yes +tab-complete.c: gen_tabcomplete.pl tab-complete.in.c + $(PERL) $^ --outfile $@ + install: all installdirs $(INSTALL_PROGRAM) psql$(X) '$(DESTDIR)$(bindir)/psql$(X)' $(INSTALL_DATA) $(srcdir)/psqlrc.sample '$(DESTDIR)$(datadir)/psqlrc.sample' @@ -75,7 +78,7 @@ uninstall: clean distclean: rm -f psql$(X) $(OBJS) lex.backup rm -rf tmp_check - rm -f sql_help.h sql_help.c psqlscanslash.c + rm -f sql_help.h sql_help.c psqlscanslash.c tab-complete.c check: $(prove_check) diff --git a/src/bin/psql/gen_tabcomplete.pl b/src/bin/psql/gen_tabcomplete.pl new file mode 100644 index 0000000000..f534ec60da --- /dev/null +++ b/src/bin/psql/gen_tabcomplete.pl @@ -0,0 +1,306 @@ +#---------------------------------------------------------------------- +# +# gen_tabcomplete.pl +# Perl script that transforms tab-complete.in.c to tab-complete.c. +# +# This script converts a C else-if chain into a switch statement. +# The else-if statements to be processed must appear at single-tab-stop +# indentation between lines reading +# /* BEGIN GEN_TABCOMPLETE */ +# /* END GEN_TABCOMPLETE */ +# The first clause in each if-condition must be a call of one of the +# functions Matches, HeadMatches, TailMatches, MatchesCS, HeadMatchesCS, +# or TailMatchesCS. Its argument(s) must be string literals or macros +# that expand to string literals or NULL. These clauses are removed from +# the code and replaced by "break; case N:", where N is a unique number +# for each such case label. +# The BEGIN GEN_TABCOMPLETE and END GEN_TABCOMPLETE lines are replaced +# by "switch (pattern_id) {" and "}" wrapping to make a valid switch. +# The remainder of the code is copied verbatim. +# +# An if-condition can also be an OR ("||") of several *Matches function +# calls, or it can be an AND ("&&") of a *Matches call with some other +# condition. For example, +# +# else if (HeadMatches("DROP", "DATABASE") && ends_with(prev_wd, '(')) +# +# will be transformed to +# +# break; +# case N: +# if (ends_with(prev_wd, '(')) +# +# In addition, there must be one input line that reads +# /* Insert tab-completion pattern data here. */ +# This line is replaced in the output file by macro calls, one for each +# replaced match condition. The output for the above example would be +# TCPAT(N, HeadMatch, "DROP", "DATABASE"), +# where N is the replacement case label, "HeadMatch" is the original +# function name shorn of "es", and the rest are the function arguments. +# The tab-completion data line must appear before BEGIN GEN_TABCOMPLETE. +# +# +# Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group +# Portions Copyright (c) 1994, Regents of the University of California +# +# src/bin/psql/gen_tabcomplete.pl +# +#---------------------------------------------------------------------- + +use strict; +use warnings FATAL => 'all'; +use Getopt::Long; + +my $outfile = ''; + +GetOptions('outfile=s' => \$outfile) or die "$0: wrong arguments"; + +open my $infh, '<', $ARGV[0] + or die "$0: could not open input file '$ARGV[0]': $!\n"; + +my $outfh; +if ($outfile) +{ + open $outfh, '>', $outfile + or die "$0: could not open output file '$outfile': $!\n"; +} +else +{ + $outfh = *STDOUT; +} + +# Opening boilerplate for output file. +printf $outfh <<EOM; +/*------------------------------------------------------------------------- + * + * tab-complete.c + * Preprocessed tab-completion code. + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * NOTES + * ****************************** + * *** DO NOT EDIT THIS FILE! *** + * ****************************** + * + * It has been GENERATED by src/bin/psql/gen_tabcomplete.pl + * + *------------------------------------------------------------------------- + */ + +#define SWITCH_CONVERSION_APPLIED + +#line 1 "tab-complete.in.c" +EOM + +# Scan input file until we find the data-replacement label line. +# Dump what we scan directly into the output file. +while (<$infh>) +{ + chomp; + last if m|^\s*/\* Insert tab-completion pattern data here\. \*/\s*$|; + print $outfh "$_\n"; +} + +# $table_data collects what we will substitute for the "pattern data" line. +my $table_data = ''; +# $output_code collects code that we can't emit till after $table_data. +my $output_code = ''; +# last case label assigned +my $last_case_label = 0; + +# We emit #line directives to keep the output file's line numbering in sync +# with the line numbering of the original, to simplify compiler error message +# reading and debugging. +my $next_line_no = $. + 1; +$output_code .= "#line ${next_line_no} \"tab-complete.in.c\"\n"; + +# Scan until we find the BEGIN GEN_TABCOMPLETE line. +# Add the scanned code to $output_code verbatim. +while (<$infh>) +{ + chomp; + last if m|^\s*/\* BEGIN GEN_TABCOMPLETE \*/\s*$|; + $output_code .= $_ . "\n"; +} + +# Emit the switch-starting lines. +$output_code .= "\tswitch (pattern_id)\n"; +$output_code .= "\t{\n"; + +# Keep line numbering in sync. +$next_line_no = $. + 1; +$output_code .= "#line ${next_line_no} \"tab-complete.in.c\"\n"; + +# Scan input file, collecting outer-level else-if conditions +# to pass to process_else_if. +# Lines that aren't else-if conditions go to $output_code verbatim. +# True if we're handling a multiline else-if condition +my $in_else_if = 0; +# The accumulated line +my $else_if_line; +my $else_if_lineno; + +while (<$infh>) +{ + chomp; + last if m|^\s*/\* END GEN_TABCOMPLETE \*/\s*$|; + if ($in_else_if) + { + my $rest = $_; + # collapse leading whitespace + $rest =~ s/^\s+//; + $else_if_line .= ' ' . $rest; + # Double right paren is currently sufficient to detect completion + if ($else_if_line =~ m/\)\)$/) + { + process_else_if($else_if_line, $else_if_lineno, $.); + $in_else_if = 0; + } + } + elsif (m/^\telse if \(/) + { + $else_if_line = $_; + $else_if_lineno = $.; + # Double right paren is currently sufficient to detect completion + if ($else_if_line =~ m/\)\)$/) + { + process_else_if($else_if_line, $else_if_lineno, $.); + } + else + { + $in_else_if = 1; + } + } + else + { + $output_code .= $_ . "\n"; + } +} + +die "unfinished else-if" if $in_else_if; + +# Emit the switch-ending lines. +$output_code .= "\tbreak;\n"; +$output_code .= "\tdefault:\n"; +$output_code .= "\t\tAssert(false);\n"; +$output_code .= "\t\tbreak;\n"; +$output_code .= "\t}\n"; + +# Keep line numbering in sync. +$next_line_no = $. + 1; +$output_code .= "#line ${next_line_no} \"tab-complete.in.c\"\n"; + +# Scan the rest, adding it to $output_code verbatim. +while (<$infh>) +{ + chomp; + $output_code .= $_ . "\n"; +} + +# Dump out the table data. +print $outfh $table_data; + +# Dump out the modified code, and we're done! +print $outfh $output_code; + +close($infh); +close($outfh); + +# Disassemble an else-if condition. +# Add the generated table-contents macro(s) to $table_data, +# and add the replacement case label(s) to $output_code. +sub process_else_if +{ + my ($else_if_line, $else_if_lineno, $end_lineno) = @_; + + # Strip the initial "else if (", which we know is there + $else_if_line =~ s/^\telse if \(//; + + # Handle OR'd conditions + my $isfirst = 1; + while ($else_if_line =~ + s/^(Head|Tail|)Matches(CS|)\((("[^"]*"|MatchAnyExcept\("[^"]*"\)|[A-Za-z,\s])+)\)\s*\|\|\s*// + ) + { + my $typ = $1; + my $cs = $2; + my $args = $3; + process_match($typ, $cs, $args, $else_if_lineno, $isfirst); + $isfirst = 0; + } + + # Check for AND'd condition + if ($else_if_line =~ + s/^(Head|Tail|)Matches(CS|)\((("[^"]*"|MatchAnyExcept\("[^"]*"\)|[A-Za-z,\s])+)\)\s*&&\s*// + ) + { + my $typ = $1; + my $cs = $2; + my $args = $3; + warn + "could not process OR/ANDed if condition at line $else_if_lineno\n" + if !$isfirst; + process_match($typ, $cs, $args, $else_if_lineno, $isfirst); + $isfirst = 0; + # approximate line positioning of AND'd condition + $output_code .= "#line ${end_lineno} \"tab-complete.in.c\"\n"; + $output_code .= "\tif ($else_if_line\n"; + } + elsif ($else_if_line =~ + s/^(Head|Tail|)Matches(CS|)\((("[^"]*"|MatchAnyExcept\("[^"]*"\)|[A-Za-z,\s])+)\)\)$// + ) + { + my $typ = $1; + my $cs = $2; + my $args = $3; + process_match($typ, $cs, $args, $else_if_lineno, $isfirst); + $isfirst = 0; + } + else + { + warn + "could not process if condition at line $else_if_lineno: the rest looks like $else_if_line\n"; + $output_code .= "\telse if ($else_if_line\n"; + } + + # Keep line numbering in sync. + if ($end_lineno != $else_if_lineno) + { + my $next_lineno = $end_lineno + 1; + $output_code .= "#line ${next_lineno} \"tab-complete.in.c\"\n"; + } +} + +sub process_match +{ + my ($typ, $cs, $args, $lineno, $isfirst) = @_; + + # Assign a new case label only for the first pattern in an OR group. + if ($isfirst) + { + $last_case_label++; + + # We intentionally keep the "break;" and the "case" on one line, so + # that they have the same line number as the original "else if"'s + # first line. This avoids misleading displays in, e.g., lcov. + $output_code .= "\t"; + $output_code .= "break; " if $last_case_label > 1; + $output_code .= "case $last_case_label:\n"; + } + + $table_data .= + "\tTCPAT(${last_case_label}, ${typ}Match${cs}, ${args}),\n"; +} + + +sub usage +{ + die <<EOM; +Usage: gen_tabcomplete.pl [--outfile/-o <path>] input_file + --outfile Output file (default is stdout) + +gen_tabcomplete.pl transforms tab-complete.in.c to tab-complete.c. +EOM +} diff --git a/src/bin/psql/meson.build b/src/bin/psql/meson.build index f3a6392138..b7c026c900 100644 --- a/src/bin/psql/meson.build +++ b/src/bin/psql/meson.build @@ -13,7 +13,6 @@ psql_sources = files( 'prompt.c', 'startup.c', 'stringutils.c', - 'tab-complete.c', 'variables.c', ) @@ -24,6 +23,17 @@ psqlscanslash = custom_target('psqlscanslash', generated_sources += psqlscanslash psql_sources += psqlscanslash +tabcomplete = custom_target('tabcomplete', + input: 'tab-complete.in.c', + output: 'tab-complete.c', + command: [ + perl, files('gen_tabcomplete.pl'), files('tab-complete.in.c'), + '--outfile', '@OUTPUT@', '@INPUT@', + ], +) +generated_sources += tabcomplete +psql_sources += tabcomplete + sql_help = custom_target('psql_help', output: ['sql_help.c', 'sql_help.h'], depfile: 'sql_help.dep', diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.in.c similarity index 99% rename from src/bin/psql/tab-complete.c rename to src/bin/psql/tab-complete.in.c index cd7640cfec..6725e15dcd 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.in.c @@ -3,7 +3,7 @@ * * Copyright (c) 2000-2024, PostgreSQL Global Development Group * - * src/bin/psql/tab-complete.c + * src/bin/psql/tab-complete.in.c * * Note: this will compile and work as-is if SWITCH_CONVERSION_APPLIED * is not defined. However, the expected usage is that it's first run -- 2.43.5
pgsql-hackers by date: