Re: BUG #18984: Empty prepared statement from psql \parse triggers assert in PortalRunMulti

On Mon, Jul 14, 2025 at 07:17:47PM -0400, Tom Lane wrote:
> I think I fat-fingered something, because now I can only reproduce back
> to v13.  Bisecting says the crash started at the v13-era commit
> 
> which is unsurprising because that's what introduced the
> Assert we're hitting.

Yep.

> which is another reason not to start throwing an error.
> (I did not look into how come that works... the correct
> command tag must be getting filled in later, but where?)

With some extra logs:
WARNING:  01000: BOOBOO exec_simple_query commandTag = EXECUTE
LOCATION:  exec_simple_query, postgres.c:1120
WARNING:  01000: BOOBOO PortalDefineQuery commandTag = EXECUTE
LOCATION:  PortalDefineQuery, portalmem.c:301
WARNING:  01000: BOOBOO PortalDefineQuery commandTag = ???
LOCATION:  PortalDefineQuery, portalmem.c:301

The first pass in PortalDefineQuery() is done in exec_simple_query().
The second pass of PortalDefineQuery() is the relevant one, I guess:
#0  PortalDefineQuery (portal=0x564cf7cc9240, prepStmtName=0x0,
sourceText=0x564cf7c74d80 "", commandTag=CMDTAG_UNKNOWN, stmts=0x0,
cplan=0x564cf7c817a8)     at portalmem.c:289
#1  0x0000564cd98d4328 in ExecuteQuery (pstate=0x564cf7c7b740,
stmt=0x564cf7c48d38, intoClause=0x0, params=0x0, dest=0x564cf7c48de0,
qc=0x7fffb8e7b4a0) at prepare.c:203
#2  0x0000564cda3f6427 in standard_ProcessUtility
(pstmt=0x564cf7c48550, queryString=0x564cf7c47590 "execute s;",
readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL,      params=0x0,
queryEnv=0x0, dest=0x564cf7c48de0, qc=0x7fffb8e7b4a0) at utility.c:759
#3  0x0000564cda3f541f in ProcessUtility (pstmt=0x564cf7c48550,
queryString=0x564cf7c47590 "execute s;", readOnlyTree=false,
context=PROCESS_UTILITY_TOPLEVEL, params=0x0,      queryEnv=0x0,
dest=0x564cf7c48de0, qc=0x7fffb8e7b4a0) at utility.c:523
#4  0x0000564cda3f2214 in PortalRunUtility (portal=0x564cf7cc9130,
pstmt=0x564cf7c48550, isTopLevel=true, setHoldSnapshot=false,
dest=0x564cf7c48de0, qc=0x7fffb8e7b4a0)     at pquery.c:1153
#5  0x0000564cda3f2a40 in PortalRunMulti (portal=0x564cf7cc9130,
isTopLevel=true, setHoldSnapshot=false, dest=0x564cf7c48de0,
altdest=0x564cf7c48de0, qc=0x7fffb8e7b4a0)     at pquery.c:1310
#6  0x0000564cda3f09a4 in PortalRun (portal=0x564cf7cc9130,
count=9223372036854775807, isTopLevel=true, dest=0x564cf7c48de0,
altdest=0x564cf7c48de0, qc=0x7fffb8e7b4a0)     at pquery.c:788
#7  0x0000564cda3e240a in exec_simple_query
(query_string=0x564cf7c47590 "execute s;") at postgres.c:1274
#8  0x0000564cda3eb78f in PostgresMain (dbname=0x564cf7c84730
"ioltas", username=0x564cf7c84718 "ioltas") at postgres.c:4767

So the QueryCompletion given to PortalRunMulti() is initialized at the
beginning of PortalRun with an unknown tag, and we miss to fill it
when running the individual queries because CopyQueryCompletion()
happens at the end of PortalRunMulti().  We can be more proactive when
updating the tag by doing one in ExecuteQuery() as you are suggesting,
but I am wondering if we should not just nuke the assertion at the end
of PortalRunMulti() instead, relying on the same check done at the
beginning of ProcessUtility().
--
Michael