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

> That guard was actually my suggestion in review, and the reason is that it’s protecting the PQescapeLiteral() calls,
notexec_query().
 
> In the RESET completion branch we call PQescapeLiteral(pset.db, ...) to safely quote the role/dbname before we even
buildthe SQL string. That happens before COMPLETE_WITH_QUERY_PLUS ever reaches complete_from_query() / exec_query(). So
whileexec_query() does guard query execution when pset.db is null or not CONNECTION_OK, it doesn’t p> revent us from
passinga null conn into PQescapeLiteral(), which could crash or otherwise misbehave.
 
> If we don’t have a usable connection, we can’t reliably quote and run the catalog query anyway, so falling back to a
staticcompletion like ALL seems like the safest behavior. Hence I think it's valid.
 

Hi,

Thank you both for clarifying my confusion. I hadn't paid much attention to `PQescapeLiteral` earlier. 
I checked the function, and it should return NULL on failure.
```
    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");
    }
```
When `pset.db` is NULL or for other reasons (resulting in q_role/q_dbname being NULL), `!q_role || !q_dbname` will be
hit— this already meets our needs. 
 
I wonder if this understanding is correct — what do you think?

--
Regards,
Man Zeng
www.openhalo.org