Re: SQL:2011 Application Time Update & Delete

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/