Re: Adding OLD/NEW support to RETURNING - Mailing list pgsql-hackers
From | jian he |
---|---|
Subject | Re: Adding OLD/NEW support to RETURNING |
Date | |
Msg-id | CACJufxHLkHOv_p-5fH9z+VrPzTpTrfs-WektxJGBaX5ya8BiHg@mail.gmail.com Whole thread Raw |
In response to | Re: Adding OLD/NEW support to RETURNING (Dean Rasheed <dean.a.rasheed@gmail.com>) |
Responses |
Re: Adding OLD/NEW support to RETURNING
|
List | pgsql-hackers |
On Sat, Jul 13, 2024 at 1:22 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > On Wed, 26 Jun 2024 at 12:18, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > > > I've added a new elog() error check to > > adjust_appendrel_attrs_mutator(), similar to the existing one for > > varnullingrels, to report if we ever attempt to apply a non-default > > varreturningtype to a non-Var, which should never be possible, but > > seems worth checking. (non-Var expressions should only occur if we've > > flattened a UNION ALL query, so shouldn't apply to the target relation > > of a data-modifying query with RETURNING.) > > > > New version attached, updating an earlier comment in > adjust_appendrel_attrs_mutator() that I had missed. > hi. I have some minor questions, but overall it just works. @@ -4884,6 +5167,18 @@ ExecEvalSysVar(ExprState *state, ExprEva { Datum d; + /* if OLD/NEW row doesn't exist, OLD/NEW system attribute is NULL */ + if ((op->d.var.varreturningtype == VAR_RETURNING_OLD && + state->flags & EEO_FLAG_OLD_IS_NULL) || + (op->d.var.varreturningtype == VAR_RETURNING_NEW && + state->flags & EEO_FLAG_NEW_IS_NULL)) + { + *op->resvalue = (Datum) 0; + *op->resnull = true; + + return; + } + in ExecEvalSysVar, we can add Asserts Assert(state->flags & EEO_FLAG_HAS_OLD || state->flags & EEO_FLAG_HAS_NEW); if I understand it correctly. in make_modifytable, contain_vars_returning_old_or_new((Node *) root->parse->returningList)) this don't need to go through the loop ``` foreach(lc, resultRelations) ``` + * In addition, the caller must provide result_relation, the index of the + * target relation for an INSERT/UPDATE/DELETE/MERGE. This is needed to + * handle any OLD/NEW RETURNING list Vars referencing target_varno. When such + * Vars are expanded, varreturningtype is copied onto any replacement Vars + * that reference result_relation. In addition, if the replacement expression + * from the targetlist is not simply a Var referencing result_relation, we + * wrap it in a ReturningExpr node, to force it to be NULL if the OLD/NEW row + * doesn't exist. + * * outer_hasSubLinks works the same as for replace_rte_variables(). */ @@ -1657,6 +1736,7 @@ typedef struct { RangeTblEntry *target_rte; List *targetlist; + int result_relation; ReplaceVarsNoMatchOption nomatch_option; int nomatch_varno; } ReplaceVarsFromTargetList_context; "to force it to be NULL if the OLD/NEW row doesn't exist." I am slightly confused. i think you mean: "to force it to be NULL if the OLD/NEW row will be resulting null." For INSERT, the old row is all null, for DELETE, the new row is all null. in sql-update.html "An unqualified column name or * causes new values to be returned. The same applies to columns qualified using the target table name or alias. " "The same", I think, refers "causes new values to be returned", but I i am not so sure. (apply to sql-insert.sql-delete, sql-merge).
pgsql-hackers by date: