Horton

Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size

Corey Huinker <corey.huinker@gmail.com>

View thread View on postgr.es
From: Corey Huinker <corey.huinker@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@postgresql.org, Nikita Glukhov <n.gluhov@postgrespro.ru>, Andrew Dunstan <andrew@dunslane.net>, David Rowley <dgrowleyml@gmail.com>
Date: 2023-02-24T09:44:03Z
Lists: 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();
          }

Commits

  1. JSON_TABLE: Add support for NESTED paths and columns

  2. Add basic JSON_TABLE() functionality

  3. Add SQL/JSON query functions

  4. Add soft error handling to some expression nodes

  5. Adjust populate_record_field() to handle errors softly

  6. Refactor code used by jsonpath executor to fetch variables

  7. Add more SQL/JSON constructor functions

  8. SQL/JSON: support the IS JSON predicate

  9. SQL/JSON: add standard JSON constructor functions

  10. Add static assertion ensuring sizeof(ExprEvalStep) <= 64 bytes

  11. Remove size increase in ExprEvalStep caused by hashed saops

  12. pgstat: reduce timer overhead by leaving timer running.

  13. expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size.

  14. SQL/JSON query functions

  15. Speedup ScalarArrayOpExpr evaluation