Japin Li <japinli@hotmail.com> writes:
> On Tue, 20 Jun 2023 at 03:00, PG Bug reporting form <noreply@postgresql.org> wrote:
>> The following psql script:
>> BEGIN;
>> ERROR;
>> ;
>> \gdesc
>> triggers an assertion failure with the following stack trace:
>> so maybe an empty input string is not legal for creating a cached plan
>> when the transaction is aborted?
> Yeah, SearchSysCache1() need in an transaction block, here is a patch
> fixed it.
I'm not sure if anyone out there is expecting that this case should
work, but it probably did work at one time. Rather than throwing
an error, it'd be better to fix plancache.c so it doesn't fail.
I looked at the code and found that that's pretty much a one-line
fix, because there are already code paths that avoid doing anything
extra for transaction control commands (e.g ROLLBACK, which'd
otherwise have this same issue). We just need to make a NULL
parsetree use those paths.
> Another question, why should we need to create a plan cache entry for
> empty input?
Well, we have to have something to support the wire protocol behavior
for this case. No doubt we could hack up postgres.c to handle it
without a plan cache entry, but it'd be far more invasive to do it
there.
regards, tom lane
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 87210fcf62..d4bfc55001 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -78,9 +78,10 @@
/*
* We must skip "overhead" operations that involve database access when the
* cached plan's subject statement is a transaction control command.
+ * For convenience, treat empty statements as control commands too.
*/
#define IsTransactionStmtPlan(plansource) \
- ((plansource)->raw_parse_tree && \
+ ((plansource)->raw_parse_tree == NULL || \
IsA((plansource)->raw_parse_tree->stmt, TransactionStmt))
/*
