On Wed, 24 Dec 2025 at 12:07, Tender Wang <tndrwang@gmail.com> wrote:
>
> I did some debugging, and I found that:
> In add_rte_to_flat_rtable(), the RTE of value was not added into glob->AllRelids, because below codes:
> .....
> the estate->es_unpruned_relids equals with result->unprunableRelids contains. So the rowMark was skipped incorrectly.
>
> I did a quick fix as the attached patch.
> Any thoughts?
Yes. However, it's not sufficient to only add RTE_VALUES RTEs to what
gets included in PlannerGlobal.allRelids. Rowmarks can be attached to
other kinds of RTEs. An obvious example is an RTE_SUBQUERY RTE that is
an actual subquery that did not come from a view. So, for this
approach to work robustly, it really should include *all* RTEs in
PlannerGlobal.allRelids.
Yes, it is. I can reproduce the same with a subquery as below:
merge into t1 using (select a from (values(1,1),(2,2) as t4(a,b)) as t3(a) on (t1.a = t3.a)
when matched then
update set b = t1.b + 1
when not matched then
insert (a,b) values (1,1);
or
merge into t1 using (select generate_series(1,2) as a) as t3 on (t1.a = t3.a)
when matched then
update set b = t1.b + 1
when not matched then
insert (a,b) values (1,1);
All reported the same error. My approach obviously can not cover all cases.
I took a slightly different approach, which was to change the test in
InitPlan() (and also in ExecInitLockRows() and ExecInitModifyTable())
to only ignore rowmarks for pruned relations that are plain
RTE_RELATION relations, since those are the only relations that are
ever actually pruned. So rowmarks attached to any other kind of RTE
are not ignored. I also added an isolation test case.
I failed to apply your patch, some errors, like "error: git apply: bad git-diff - expected /dev/null on line 2"
or "Patch format detection failed."
I looked through the patch. It worked for me.
I'm somewhat conflicted as to which approach is better. I think maybe
there is less chance of other unintended side-effects if the set of
RTEs included in PlannerGlobal.allRelids, unprunableRelids, and
es_unpruned_relids is not changed. However, as it stands,
PlannerGlobal.allRelids is misnamed (it should probably have been
called "relationRelids", in line with the "relationOids" field).
Making it actually include all RTEs would solve that.
.....
/*
* RT indexes of all relation RTEs in finalrtable (RTE_RELATION and
* RTE_SUBQUERY RTEs of views)
*/
......
Per the comment, I guess Amit didn't plan to include all RTEs in his design.
