Re: SQL:2011 Application Time Update & Delete - Mailing list pgsql-hackers

From jian he
Subject Re: SQL:2011 Application Time Update & Delete
Date
Msg-id CACJufxHALFKca5SMn5DNnbrX2trPamVL6napn_nm35p15yw+rg@mail.gmail.com
Whole thread Raw
In response to Re: SQL:2011 Application Time Update & Delete  (Paul A Jungwirth <pj@illuminatedcomputing.com>)
List pgsql-hackers
hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f

DROP TABLE If EXISTS tt;
CREATE TABLE tt(id int, valid_at int4range, amt int, CONSTRAINT
fpo2_check CHECK (upper(valid_at) <> '11'));
CREATE OR REPLACE FUNCTION dummy_update_func() RETURNS trigger AS $$
BEGIN
  RAISE NOTICE 'dummy_update_func(%) called: action = %, old = %, new = %',
    TG_ARGV[0], TG_OP, OLD, NEW;
  RETURN NEW;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER some_trig_before BEFORE UPDATE OR INSERT ON tt FOR EACH
ROW EXECUTE PROCEDURE dummy_update_func('before');
INSERT INTO tt VALUES (1, '[1,100)', 2);
UPDATE tt FOR PORTION OF valid_at FROM 1 TO 12 SET amt = 3;

NOTICE:  dummy_update_func(before) called: action = UPDATE, old =
(1,"[1,100)",2), new = (1,"[1,12)",3)
NOTICE:  dummy_update_func(before) called: action = INSERT, old =
<NULL>, new = (1,"[12,100)",2)

As you can see, ExecGetAllUpdatedCols does not account for the valid_at column,
even though it is actively being updated. ExecGetAllUpdatedCols is being used
serval places, IMHO, we need to add some comments on
ExecGetAllUpdatedCols explaining
this behavior and maybe add some regression tests.

I'm not sure if it's safe for ExecGetAllUpdatedCols to ignore the FOR
PORTION OF column.

I reliazed this issue because of https://commitfest.postgresql.org/patch/6270/
I saw your transformForPortionOfClause comments.
        /*
         * The range column will change, but you don't need UPDATE permission
         * on it, so we don't add to updatedCols here. XXX: If
         *
https://www.postgresql.org/message-id/CACJufxEtY1hdLcx%3DFhnqp-ERcV1PhbvELG5COy_CZjoEW76ZPQ%40mail.gmail.com
         * is merged (only validate CHECK constraints if they depend on one of
         * the columns being UPDATEd), we need to make sure that code knows
         * that we are updating the application-time column.
         */
But this comment is about FOR PORTION OF column permission, not about
ExecGetAllUpdatedCols.

-------------------------------------------------------------------------------------------------------------------
transformForPortionOfClause
    if (contain_volatile_functions_after_planning((Expr *) result->targetRange))
        ereport(ERROR,
                (errmsg("FOR PORTION OF bounds cannot contain volatile
functions")));

Need
errcode(ERRCODE_FEATURE_NOT_SUPPORTED).

coerce_to_target_type function comment:
 * This is the general-purpose entry point for arbitrary type coercion
 * operations.  Direct use of the component operations can_coerce_type,
 * coerce_type, and coerce_type_typmod should be restricted to special
 * cases (eg, when the conversion is expected to succeed).

We should use coerce_to_target_type more, not can_coerce_type,
coerce_type individually.
coerce_to_target_type also handles `UNKNOWN` constant, which ensures
the deparsing casts to the correct data type.

please see the attached refactoring for
https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f



--
jian
https://www.enterprisedb.com/

Attachment

pgsql-hackers by date:

Previous
From: Xuneng Zhou
Date:
Subject: Re: Implement waiting for wal lsn replay: reloaded
Next
From: Michael Paquier
Date:
Subject: Re: Adding locks statistics