From 5bceee66c3ec4e4c271dddccedff62ba8ea16a57 Mon Sep 17 00:00:00 2001 From: Ishjaap Singh Date: Sat, 31 Oct 2026 01:56:51 +0530 Subject: [PATCH v1] psql: Don't offer USING INDEX after REPACK (CONCURRENTLY) or (ANALYZE) After a REPACK option list, tab completion offered USING INDEX along with the relation names. Without a table name, REPACK USING INDEX processes every eligible table with a clustering index, which is never allowed with CONCURRENTLY or ANALYZE, so choosing it always fails: REPACK (CONCURRENTLY) USING INDEX; ERROR: REPACK (CONCURRENTLY) requires an explicit table name REPACK (ANALYZE) USING INDEX; ERROR: cannot execute REPACK (ANALYZE) on multiple tables Read the completed option list and leave out USING INDEX when CONCURRENTLY or ANALYZE (also spelled ANALYSE) ends up enabled. As on the server, the last setting of an option wins. Only option names and values written as plain unquoted words are interpreted; if the list contains anything else, such as a quoted name or value, the existing completions are kept. --- src/bin/psql/t/010_tab_completion.pl | 55 +++++++++++++ src/bin/psql/tab-complete.in.c | 117 ++++++++++++++++++++++++++- 2 files changed, 170 insertions(+), 2 deletions(-) diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl index 1d2e5f5b92a..4031ad19278 100644 --- a/src/bin/psql/t/010_tab_completion.pl +++ b/src/bin/psql/t/010_tab_completion.pl @@ -422,6 +422,61 @@ check_completion( clear_line(); +# check REPACK option lists. CONCURRENTLY and ANALYZE need a table name, so +# USING INDEX must not be offered after them. The table name below shares a +# prefix with USING INDEX: if only the table matches, "us" completes to it. +$node->safe_psql('postgres', "CREATE TABLE using_tab (f1 int);"); + +foreach my $opts ( + '(CONCURRENTLY)', + '(analyze)', + '(concurrently true)', + '(verbose, concurrently 1)', + '(concurrently off, concurrently)', + '(analyse off, analyze on)') +{ + check_completion( + "REPACK $opts us\t", + qr/using_tab /, + "REPACK $opts does not offer USING INDEX"); + + clear_query(); +} + +# In these cases both USING INDEX and the table match, so the first tab +# stops at the shared prefix and a second double tab lists both. The last +# three have values psql doesn't interpret, so USING INDEX stays, even after +# an earlier setting that turned CONCURRENTLY on. +foreach my $cmd ( + 'REPACK', + 'REPACK (verbose)', + 'REPACK (CONCURRENTLY OFF)', + 'REPACK (concurrently, concurrently false)', + 'REPACK (analyze, analyse 0)', + 'REPACK (concurrently, concurrently maybe)', + "REPACK (concurrently 'on')", + "REPACK (concurrently, concurrently 'off')") +{ + check_completion("$cmd us\t", qr/using/, "$cmd completes common prefix"); + check_completion( + "\t\t", + qr/using index +using_tab/, + "$cmd still offers USING INDEX"); + + clear_query(); +} + +# With an explicit table, USING INDEX is fine again. +foreach my $opts ('(CONCURRENTLY)', '(analyze)') +{ + check_completion( + "REPACK $opts tab1 us\t", + qr/using index /i, + "REPACK $opts with a table offers USING INDEX"); + + clear_query(); +} + # send psql an explicit \q to shut it down, else pty won't close properly $h->quit or die "psql returned $?"; diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index 96de78cd087..ef555866036 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -1837,6 +1837,110 @@ ends_with(const char *s, char c) return (length > 0 && s[length - 1] == c); } +/* + * Read a plain word (ASCII letters, digits and underscores) at *p into buf, + * downcased the way the server downcases unquoted identifiers, and advance + * *p past it. Returns false if there is no such word or it doesn't fit. + */ +static bool +read_option_word(const char **p, char *buf, size_t bufsize) +{ + size_t len = strspn(*p, "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_"); + + if (len == 0 || len >= bufsize) + return false; + for (size_t i = 0; i < len; i++) + buf[i] = pg_ascii_tolower((unsigned char) (*p)[i]); + buf[len] = '\0'; + *p += len; + return true; +} + +/* + * Given a completed parenthesized option list for REPACK, such as + * "(CONCURRENTLY, VERBOSE OFF)", report whether it turns on an option that + * makes the server require an explicit table name: CONCURRENTLY or ANALYZE + * (also spelled ANALYSE). As on the server, the last setting of an option + * wins. + * + * Only option names and values written as plain unquoted words are + * understood. Anything else (quoted names or values, signed numbers, + * comments, ...) makes us return false, so that the caller keeps offering + * every completion that might be valid. + */ +static bool +repack_options_require_table(const char *options) +{ + const char *p = options; + bool concurrently = false; + bool analyze = false; + + if (*p++ != '(') + return false; + + for (;;) + { + char name[NAMEDATALEN]; + char value[NAMEDATALEN]; + bool has_value; + bool *target; + + /* an option name is an identifier, so it can't start with a digit */ + while (isspace((unsigned char) *p)) + p++; + if (isdigit((unsigned char) *p) || + !read_option_word(&p, name, sizeof(name))) + return false; + + while (isspace((unsigned char) *p)) + p++; + has_value = (*p != ',' && *p != ')'); + if (has_value) + { + if (!read_option_word(&p, value, sizeof(value))) + return false; + while (isspace((unsigned char) *p)) + p++; + if (*p != ',' && *p != ')') + return false; + } + + if (strcmp(name, "concurrently") == 0) + target = &concurrently; + else if (strcmp(name, "analyze") == 0 || strcmp(name, "analyse") == 0) + target = &analyze; + else + target = NULL; /* an option we don't care about */ + + if (target != NULL) + { + /* Interpret the value the way defGetBoolean() does */ + if (!has_value || + strcmp(value, "true") == 0 || + strcmp(value, "on") == 0 || + strcmp(value, "1") == 0) + *target = true; + else if (strcmp(value, "false") == 0 || + strcmp(value, "off") == 0 || + strcmp(value, "0") == 0) + *target = false; + else + return false; + } + + if (*p++ == ')') + break; + } + + /* The option list must end here */ + if (*p != '\0') + return false; + + return concurrently || analyze; +} + /* * The completion function. * @@ -5127,8 +5231,17 @@ match_previous_words(int pattern_id, COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_clusterables, "(", "USING INDEX"); else if (Matches("REPACK", "(*)")) - COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_clusterables, - "USING INDEX"); + { + /* + * REPACK (CONCURRENTLY) and REPACK (ANALYZE) need a table name, so + * don't offer USING INDEX right away in those cases. + */ + if (repack_options_require_table(prev_wd)) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables); + else + COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_clusterables, + "USING INDEX"); + } else if (Matches("REPACK", MatchAnyExcept("("))) COMPLETE_WITH("USING INDEX"); else if (Matches("REPACK", "(*)", MatchAnyExcept("("))) -- 2.49.0.windows.1