Incorrect assertion in ExecBuildAggTrans ?
Andrew Gierth <andrew@tao11.riddles.org.uk>
From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: pgsql-hackers@lists.postgresql.org
Date: 2020-12-21T12:02:16Z
Lists: pgsql-hackers
ExecBuildAggTrans has this sequence:
if (pertrans->deserialfn.fn_strict)
scratch.opcode = EEOP_AGG_STRICT_DESERIALIZE;
else
scratch.opcode = EEOP_AGG_DESERIALIZE;
scratch.d.agg_deserialize.fcinfo_data = ds_fcinfo;
scratch.d.agg_deserialize.jumpnull = -1; /* adjust later */
scratch.resvalue = &trans_fcinfo->args[argno + 1].value;
scratch.resnull = &trans_fcinfo->args[argno + 1].isnull;
ExprEvalPushStep(state, &scratch);
adjust_bailout = lappend_int(adjust_bailout,
state->steps_len - 1);
but later on, where adjust_bailout is processed, we see this (note that
EEOP_AGG_DESERIALIZE is not checked for):
if (as->opcode == EEOP_JUMP_IF_NOT_TRUE)
{
Assert(as->d.jump.jumpdone == -1);
as->d.jump.jumpdone = state->steps_len;
}
else if (as->opcode == EEOP_AGG_STRICT_INPUT_CHECK_ARGS ||
as->opcode == EEOP_AGG_STRICT_INPUT_CHECK_NULLS)
{
Assert(as->d.agg_strict_input_check.jumpnull == -1);
as->d.agg_strict_input_check.jumpnull = state->steps_len;
}
else if (as->opcode == EEOP_AGG_STRICT_DESERIALIZE)
{
Assert(as->d.agg_deserialize.jumpnull == -1);
as->d.agg_deserialize.jumpnull = state->steps_len;
}
else
Assert(false);
Seems clear to me that the assertion is wrong, and that even though a
non-strict DESERIALIZE opcode might not need jumpnull filled in, the
code added it to adjust_bailout anyway, so we crash out here on an
asserts build. This may have been overlooked because all the builtin
deserialize functions appear to be strict, but postgis has at least one
non-strict one and can hit this.
This could be fixed either by fixing the assert, or by not adding
non-strict deserialize opcodes to adjust_bailout; anyone have any
preferences?
--
Andrew (irc:RhodiumToad)
Commits
-
Don't add bailout adjustment for non-strict deserialize calls.
- f7f2a28d4350 11.11 landed
- 30f912a41378 12.6 landed
- 75e3cca42d6f 13.2 landed
- a3367aa3c455 14.0 landed