Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size - Mailing list pgsql-hackers

From Corey Huinker
Subject Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size
Date
Msg-id CADkLM=fs7=X1iNhNXnH117Z39kQMzVW=5k2AGT9rQ-WSfpheqw@mail.gmail.com
Whole thread Raw
In response to Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Thu, Feb 23, 2023 at 2:39 PM Andres Freund <andres@anarazel.de> wrote:
Hi,

On 2023-02-23 13:56:56 -0500, Tom Lane wrote:
> Corey Huinker <corey.huinker@gmail.com> writes:
> > My not-ready-for-16 work on CAST( ... ON DEFAULT ... ) involved making
> > FuncExpr/IoCoerceExpr/ArrayCoerceExpr have a safe_mode flag, and that
> > necessitates adding a reserror boolean to ExprEvalStep for subsequent steps
> > to test if the error happened.
>
> Why do you want it in ExprEvalStep ... couldn't it be in ExprState?
> I can't see why you'd need more than one at a time during evaluation.

I don't know exactly what CAST( ... ON DEFAULT ... ) is aiming for - I guess
it wants to assign a different value when the cast fails?  Is the default
expression a constant, or does it need to be runtime evaluated?  If a const,
then the cast steps just could assign the new value. If runtime evaluation is
needed I'd expect the various coerce steps to jump to the value implementing
the default expression in case of a failure.

The default expression is itself a cast expression. So CAST (expr1 AS some_type DEFAULT expr2 ON ERROR) would basically be a safe-mode cast of expr1 to some_type, and only upon failure would the non-safe cast of expr2 to some_type be executed. Granted, the most common use case would be for expr2 to be a constant or something that folds into a constant, but the proposed spec allows for it.

My implementation involved adding a setting to CoalesceExpr that tested for error flags rather than null flags, hence putting it in ExprEvalStep and ExprState (perhaps mistakenly). Copying and adapting EEOP_JUMP_IF_NOT_NULL lead me to this:

          EEO_CASE(EEOP_JUMP_IF_NOT_ERROR)
          {    
              /* Transfer control if current result is non-error */
              if (!*op->reserror)
              {    
                  *op->reserror = false;
                  EEO_JUMP(op->d.jump.jumpdone);
              }    
 
              /* reset error flag */
              *op->reserror = false;
 
              EEO_NEXT();
          }    


pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: Doc updates for MERGE
Next
From: Daniel Gustafsson
Date:
Subject: Re: TAP output format in pg_regress