Re: [PATCH] psql: tab completion for ALTER ROLE ... IN DATABASE ...

On 2026-Jan-06, VASUKI M wrote:

> Sorry,it was an incremental patch.
> The attached v5 is the fresh patch against Master.

I have pushed this, but I felt the completion for RESET was not ready
yet, so I left it out.  I also removed a bunch of useless comments and
braces and stuff that I felt added no value.  (LLM-generated code should
not be treated as being in final submittable state -- you need to
examine it critically and remove the crap.)

On this part:

> +    /* ALTER USER/ROLE <name> IN DATABASE <dbname> RESET */
> +    else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "RESET"))
> +    {
> +        if (!pset.db || PQstatus(pset.db) != CONNECTION_OK)
> +        {
> +            COMPLETE_WITH("ALL");
> +        }
> +        else
> +        {
> +            /*
> +             * Extract tokens: prev5 = role name prev2 = database name
> +             */
> +            char       *role = prev5_wd;
> +            char       *dbname = prev2_wd;
> +            char       *q_role;
> +            char       *q_dbname;
> +            char       *query;
> +
> +            /* Safe SQL literal quoting using libpq */
> +            q_role = PQescapeLiteral(pset.db, role, strlen(role));
> +            q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
> +            if (!q_role || !q_dbname)
> +            {
> +                /* If quoting fails, just fall back to ALL */
> +                if (q_role)
> +                    PQfreemem(q_role);
> +                if (q_dbname)
> +                    PQfreemem(q_dbname);
> +                COMPLETE_WITH("ALL");
> +            }
> +            else
> +            {
> +                query = psprintf(
> +                                 " SELECT split_part(unnest(setconfig), \'=\', 1) "
> +                                 "  FROM pg_db_role_setting "
> +                                 " WHERE setdatabase = "
> +                                 "       (SELECT oid FROM pg_database WHERE datname = %s) "
> +                                 "   AND setrole = %s::regrole",
> +                                 q_dbname, q_role);
> +                COMPLETE_WITH_QUERY_PLUS(query, "ALL");
> +                PQfreemem(q_role);
> +                PQfreemem(q_dbname);
> +                pfree(query);
> +            }
> +        }
> +    }

Nice attempt, not yet ready.

There's nowhere else in match_previous_words() that does anything this
complicated, and I didn't want to add the first one, because this occurs
between the GEN_TABCOMPLETE markers, which means the code is going to be
rewritten by a Perl script -- here be dragons -- happy to leave it
undisturbed.  If we need this complexity, let's create a separate
function for it.

Speaking of functions -- there are some function calls in that query,
and they are not schema-qualified.  That's unacceptable in psql.  Add
pg_catalog. schema quals there, like all other queries in that file.

pfree() looks out of place in psql.  I would use stock free(), like we
do for the pg_strdup() calls.

I'm unsure about this novel use of COMPLETE_WITH_QUERY.  It seems if we
have
  ALTER ROLE foo IN DATABASE bar RESET enable<tab>
then we'll dismiss the "enable" part if we have multiple variables set
that start with "enable", which is not very nice.  (I checked -- yes, it's
broken).  Maybe we need a new generator for COMPLETE_WITH_GENERATOR, or
maybe we need more features in complete_from_query().


Anyway, this is pretty much what Ian submitted at the start of the
thread, so I've set it as committed in the CF app.

Thanks!

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/