> That guard was actually my suggestion in review, and the reason is that it??s
> protecting the PQescapeLiteral() calls, not exec_query().
> In the RESET completion branch we call PQescapeLiteral(pset.db, ...) to
> safely quote the role/dbname before we even build the SQL string. That
> happens before COMPLETE_WITH_QUERY_PLUS ever reaches complete_from_query() /
> exec_query(). So while exec_query() does guard query execution when pset.db
> is null or not CONNECTION_OK, it doesn??t p> revent us from passing a 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 static completion 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
