Incorrect assertion in ExecBuildAggTrans ?

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Incorrect assertion in ExecBuildAggTrans ?
Date: 2020-12-21 12:02:16
Message-ID: 87mty7peb3.fsf@news-spur.riddles.org.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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)

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-12-21 12:38:02 Re: Single transaction in the tablesync worker?
Previous Message Maxim Orlov 2020-12-21 10:49:25 Re: [PATCH] Automatic HASH and LIST partition creation