Incorrect assertion in ExecBuildAggTrans ?

Поиск
Список
Период
Сортировка
От Andrew Gierth
Тема Incorrect assertion in ExecBuildAggTrans ?
Дата
Msg-id 87mty7peb3.fsf@news-spur.riddles.org.uk
обсуждение исходный текст
Список 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)



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Maxim Orlov
Дата:
Сообщение: Re: [PATCH] Automatic HASH and LIST partition creation
Следующее
От: Amit Kapila
Дата:
Сообщение: Re: Single transaction in the tablesync worker?