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/
