> 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
