Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Repair commits 317aba70e et al for -DWRITE_READ_PARSE_PLAN_TREES.

  2. Build whole-row Vars the same way during parsing and planning.

  3. Preserve RangeTblEntry.relid when expanding a view RTE.

  4. Fix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY SOURCE.

  1. Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Duncan Sands <duncan.sands@deepbluecap.com> — 2025-03-10T09:48:34Z

    Postgresql version 17.4 (Ubuntu 17.4-1.pgdg24.10+2) on x86_64-pc-linux-gnu

To reproduce, execute the statements in the attached file cr.sql.  I get:

duncan=> \i cr.sql
CREATE TABLE
CREATE TABLE
CREATE VIEW
CREATE TABLE
COPY 1
COPY 2
COPY 1
psql:cr.sql:42: ERROR:  attribute 2 of type record has wrong type
DETAIL:  Table has type _country_or_region, but query expects record.

I attribute it to the "WHEN NOT MATCHED BY SOURCE THEN DELETE" part of the MERGE 
as it doesn't happen if that part is left off.

Best wishes, Duncan.

  2. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tender Wang <tndrwang@gmail.com> — 2025-03-10T12:32:40Z

    Duncan Sands <duncan.sands@deepbluecap.com> 于2025年3月10日周一 18:43写道：

> Postgresql version 17.4 (Ubuntu 17.4-1.pgdg24.10+2) on x86_64-pc-linux-gnu
>
> To reproduce, execute the statements in the attached file cr.sql.  I get:
>
> duncan=> \i cr.sql
> CREATE TABLE
> CREATE TABLE
> CREATE VIEW
> CREATE TABLE
> COPY 1
> COPY 2
> COPY 1
> psql:cr.sql:42: ERROR:  attribute 2 of type record has wrong type
> DETAIL:  Table has type _country_or_region, but query expects record.
>
> I attribute it to the "WHEN NOT MATCHED BY SOURCE THEN DELETE" part of the
> MERGE
> as it doesn't happen if that part is left off.
>
>
Yeah, I can reproduce this on HEAD, but on 17.0, no error happened.
I searched commit history, I found that this error was related to d7d297f84.

commit d7d297f8449641bfd71750d04c302572a350052c
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Date:   Thu Oct 3 12:50:38 2024 +0100

    Fix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY
SOURCE.

    If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the
    merge join condition is used by the executor to distinguish MATCHED
    from NOT MATCHED BY SOURCE cases. However, this qual is executed using
    the output from the join subplan node, which nulls the output from the
    source relation in the not matched case, and so the result may be
    incorrect if the join condition is "non-strict" -- for example,
    something like "src.col IS NOT DISTINCT FROM tgt.col".

Reverted above commit, cr.sql succeeded.

 psql (17.0)
Type "help" for help.

postgres=# \i /workspace/cr.sql
CREATE TABLE
CREATE TABLE
CREATE VIEW
CREATE TABLE
COPY 1
COPY 2
COPY 1
MERGE 0


-- 
Thanks,
Tender Wang

  3. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tender Wang <tndrwang@gmail.com> — 2025-03-10T13:46:36Z

    Duncan Sands <duncan.sands@deepbluecap.com> 于2025年3月10日周一 18:43写道：

> Postgresql version 17.4 (Ubuntu 17.4-1.pgdg24.10+2) on x86_64-pc-linux-gnu
>
> To reproduce, execute the statements in the attached file cr.sql.  I get:
>
> duncan=> \i cr.sql
> CREATE TABLE
> CREATE TABLE
> CREATE VIEW
> CREATE TABLE
> COPY 1
> COPY 2
> COPY 1
> psql:cr.sql:42: ERROR:  attribute 2 of type record has wrong type
> DETAIL:  Table has type _country_or_region, but query expects record.
>
> I attribute it to the "WHEN NOT MATCHED BY SOURCE THEN DELETE" part of the
> MERGE
> as it doesn't happen if that part is left off.


When the query has NOT MATCHED BY SOURCE, commit d7d297f84 add "src IS NOT
NULL" join condition.
In this case, the src is view(e.g. subquery), so in makeWholeRowVar(), it
will call below code:
result = makeVar(varno,
                      InvalidAttrNumber,
                      RECORDOID,
                      -1,
                      InvalidOid,
                      varlevelsup);

the vartype is RECORDOID, but te reltype of src is not RECORDOID, so
$SUBJECT error reports.

I add the below codes to makeWholeRowVar() default branch:

if (rte->relkind == RELKIND_VIEW)
        toid = get_rel_type_id(rte->relid);
else
        toid = RECORDOID;

It can work.


-- 
Thanks,
Tender Wang

  4. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-10T15:00:46Z

    Tender Wang <tndrwang@gmail.com> writes:
> When the query has NOT MATCHED BY SOURCE, commit d7d297f84 add "src IS NOT
> NULL" join condition.
> In this case, the src is view(e.g. subquery), so in makeWholeRowVar(), it
> will call below code:
> result = makeVar(varno,
>                       InvalidAttrNumber,
>                       RECORDOID,
>                       -1,
>                       InvalidOid,
>                       varlevelsup);

> the vartype is RECORDOID, but te reltype of src is not RECORDOID, so
> $SUBJECT error reports.

Hmm.  I tried adjusting the example to make _country_or_region
be a materialized view or plain table instead of a view, and
those cases did not fail.  I wonder why the difference...

> I add the below codes to makeWholeRowVar() default branch:
> if (rte->relkind == RELKIND_VIEW)
>         toid = get_rel_type_id(rte->relid);
> else
>         toid = RECORDOID;
> It can work.

I'm quite uncomfortable with the idea of changing makeWholeRowVar()
itself in this way --- the potential blast radius from that seems
rather large.  A localized fix in transform_MERGE_to_join() might
be a wiser answer.

			regards, tom lane

  5. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-10T15:11:16Z

    On Mon, 10 Mar 2025 at 13:46, Tender Wang <tndrwang@gmail.com> wrote:
>
> When the query has NOT MATCHED BY SOURCE, commit d7d297f84 add "src IS NOT NULL" join condition.
> In this case, the src is view(e.g. subquery), so in makeWholeRowVar(), it will call below code:
> result = makeVar(varno,
>                       InvalidAttrNumber,
>                       RECORDOID,
>                       -1,
>                       InvalidOid,
>                       varlevelsup);
>
> the vartype is RECORDOID, but te reltype of src is not RECORDOID, so $SUBJECT error reports.
>
> I add the below codes to makeWholeRowVar() default branch:
>
> if (rte->relkind == RELKIND_VIEW)
>         toid = get_rel_type_id(rte->relid);
> else
>         toid = RECORDOID;
>
> It can work.
>

Yes, I reached the same conclusion.

When the parser processes the "AND qq_src IS DISTINCT FROM qq_tgt"
clause, it creates a whole-row Var for qq_src whose type is the view
type. Then transform_MERGE_to_join() adds another whole-row Var for
qq_src, but by this time the RTE has been expanded into a subquery
RTE, so its type becomes RECORDOID. The executor then grumbles because
it has 2 Vars with the same varno and varattno, but different
vartypes.

Fixing that by having makeWholeRowVar() set the type based on
rte->relid for subquery RTEs that used to be views seems like a good
fix. However, it looks like that fix will only work as far back as v16
(where 47bb9db7599 and 0f8cfaf8921 were added).

Unfortunately, it looks like this bug pre-dates MERGE WHEN NOT MATCHED
BY SOURCE, and even MERGE itself. All that's needed to trigger it is a
query that causes 2 whole-row Vars to be added, one before and one
after view expansion. That can be made to happen via the rowmarking
mechanism in all supported branches as follows:

create table foo (a int, b int);
create view foo_v as select * from foo offset 0;
insert into foo values (1,2);
update foo set b = foo_v.b from foo_v where foo_v.a = foo.a returning foo_v;

which fails in the same way, with

ERROR:  attribute 3 of type record has wrong type
DETAIL:  Table has type record, but query expects foo_v.

Reading the commit message for 47bb9db7599 suggests that maybe it
would be OK to further back-patch the changes to ApplyRetrieveRule()
to retain relkind and relid on subquery RTEs for this purpose. That
wouldn't affect stored rules, but I haven't looked to see what else it
might affect.

Thoughts?

Regards,
Dean

  6. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-10T17:52:42Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> Unfortunately, it looks like this bug pre-dates MERGE WHEN NOT MATCHED
> BY SOURCE, and even MERGE itself. All that's needed to trigger it is a
> query that causes 2 whole-row Vars to be added, one before and one
> after view expansion. That can be made to happen via the rowmarking
> mechanism in all supported branches as follows:

Ugh, right.  So I withdraw my objection to fixing this in
makeWholeRowVar: all of the post-rewrite calls have need for this
behavior.  However, the proposed code change is wrong in detail.
The existing places that are checking for this situation are doing
tests like
	(rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid)))
I don't believe that checking relkind instead is an improvement.

> Reading the commit message for 47bb9db7599 suggests that maybe it
> would be OK to further back-patch the changes to ApplyRetrieveRule()
> to retain relkind and relid on subquery RTEs for this purpose. That
> wouldn't affect stored rules, but I haven't looked to see what else it
> might affect.

Yeah, I think we can likely get away with that.  We cannot back-patch
the changes that added relid to the outfuncs/readfuncs representation,
which means that the RTE's relid won't propagate to parallel workers,
but I don't see why they'd need it.  We only need that info to get
as far as planning.  I've not tried though.

Draft HEAD patch attached.

			regards, tom lane

  7. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-10T19:11:20Z

    I wrote:
> Yeah, I think we can likely get away with that.  We cannot back-patch
> the changes that added relid to the outfuncs/readfuncs representation,
> which means that the RTE's relid won't propagate to parallel workers,
> but I don't see why they'd need it.  We only need that info to get
> as far as planning.  I've not tried though.

OK, the attached patch for v15 passes check-world, with or without
force_parallel_mode.  I'm inclined to commit the rewriteHandler.c
and parsenodes.h bits in a separate patch for commit log visibility.

It would be easy enough to leave the RTE's relkind and/or rellockmode
alone too, but I think the conservative approach is to not change
more than we have to in these old branches.  There is some attraction
to making the behavior more like the newer branches, but still ...

			regards, tom lane

  8. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-10T20:03:15Z

    On Mon, 10 Mar 2025 at 19:11, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > Yeah, I think we can likely get away with that.  We cannot back-patch
> > the changes that added relid to the outfuncs/readfuncs representation,
> > which means that the RTE's relid won't propagate to parallel workers,
> > but I don't see why they'd need it.  We only need that info to get
> > as far as planning.  I've not tried though.
>
> OK, the attached patch for v15 passes check-world, with or without
> force_parallel_mode.  I'm inclined to commit the rewriteHandler.c
> and parsenodes.h bits in a separate patch for commit log visibility.
>

That looks good to me, on a quick read-through.

However, that's not quite the end of it -- preprocess_function_rtes()
/ inline_set_returning_function() can turn a function RTE into a
subquery RTE, leading to a similar problem:

create table foo (a int, b int);
insert into foo values (1,2);
create or replace function f() returns setof foo as
  $$ select * from foo offset 0 $$ language sql stable;
update foo set b = f.b from f() as f(a,b) where f.a = foo.a returning f;

ERROR:  attribute 3 of type record has wrong type
DETAIL:  Table has type record, but query expects foo.

I tried looking for other places that change an RTE's rtekind, and
haven't managed to find any other problems, but may have missed
something.

Regards,
Dean

  9. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-10T20:08:46Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> However, that's not quite the end of it -- preprocess_function_rtes()
> / inline_set_returning_function() can turn a function RTE into a
> subquery RTE, leading to a similar problem:

> create table foo (a int, b int);
> insert into foo values (1,2);
> create or replace function f() returns setof foo as
>   $$ select * from foo offset 0 $$ language sql stable;
> update foo set b = f.b from f() as f(a,b) where f.a = foo.a returning f;

> ERROR:  attribute 3 of type record has wrong type
> DETAIL:  Table has type record, but query expects foo.

Double ugh.  I guess we could get preprocess_function_rtes to
insert the appropriate relid ...

			regards, tom lane

  10. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-10T20:20:20Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> I tried looking for other places that change an RTE's rtekind, and
> haven't managed to find any other problems, but may have missed
> something.

The only other place I can find that converts some other kind of RTE
to a subquery is inline_cte(), which converts RTE_CTE to RTE_SUBQUERY.
That seems immune to the present problem because the appropriate
vartype would be RECORD in either case.

			regards, tom lane

  11. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tender Wang <tndrwang@gmail.com> — 2025-03-11T06:11:47Z

    Tom Lane <tgl@sss.pgh.pa.us> 于2025年3月11日周二 03:11写道：

> I wrote:
> > Yeah, I think we can likely get away with that.  We cannot back-patch
> > the changes that added relid to the outfuncs/readfuncs representation,
> > which means that the RTE's relid won't propagate to parallel workers,
> > but I don't see why they'd need it.  We only need that info to get
> > as far as planning.  I've not tried though.
>
> OK, the attached patch for v15 passes check-world, with or without
> force_parallel_mode.  I'm inclined to commit the rewriteHandler.c
> and parsenodes.h bits in a separate patch for commit log visibility.
>

The attached patch looks better than my proposed code.
LGTM.


-- 
Thanks,
Tender Wang

  12. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-11T17:14:31Z

    I wrote:
> Double ugh.  I guess we could get preprocess_function_rtes to
> insert the appropriate relid ...

OK, that was less painful than I feared.  makeWholeRowVar has
several different special cases for RTE_FUNCTION, but most of them
don't bear on this problem, because we wouldn't have applied inlining
when they did.  It seems sufficient to fetch pg_type.typrelid for
the function's nominal return type and store that if it's not 0.

Patches for HEAD and v15 attached.  I haven't checked other branches
but I expect the deltas won't be big.

			regards, tom lane

  13. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-11T19:20:54Z

    On Tue, 11 Mar 2025 at 17:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > Double ugh.  I guess we could get preprocess_function_rtes to
> > insert the appropriate relid ...
>
> OK, that was less painful than I feared.  makeWholeRowVar has
> several different special cases for RTE_FUNCTION, but most of them
> don't bear on this problem, because we wouldn't have applied inlining
> when they did.  It seems sufficient to fetch pg_type.typrelid for
> the function's nominal return type and store that if it's not 0.
>

Hmm, this introduces a new problem. Testing a case with a composite type:

create table foo (a int, b int);
insert into foo values (1,2);
create type t as (a int, b int);
create or replace function f() returns setof t as
  $$ select 1,2 from foo offset 0 $$ language sql stable;

then doing

update foo set b = f.b from f() where f.a = foo.a returning f;

or even just

select f from f();

triggers an Assert() in relation_open() from
get_relation_data_width(), from set_rel_width() because now that the
RTE has a relid, it attempts to open it, without having previously
locked it. Maybe we need to not clear rte->functions, and use that
instead.

Regards,
Dean

  14. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-11T20:06:04Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> Hmm, this introduces a new problem. Testing a case with a composite type:

> create table foo (a int, b int);
> insert into foo values (1,2);
> create type t as (a int, b int);
> create or replace function f() returns setof t as
>   $$ select 1,2 from foo offset 0 $$ language sql stable;
> then doing
> update foo set b = f.b from f() where f.a = foo.a returning f;
> or even just
> select f from f();
> triggers an Assert() in relation_open() from
> get_relation_data_width(), from set_rel_width() because now that the
> RTE has a relid, it attempts to open it, without having previously
> locked it.

Geez, our regression tests seem quite lacking in this area.

I'm wondering why we're trying to do relation_open on a SUBQUERY
RTE, relid or no relid.  It seems kind of accidental that
set_rel_width() doesn't fail on subqueries given this coding.
Even without actual failure, looking to the referenced relation for
width estimates for a subquery seems like a pretty broken idea.
However, this does indicate that putting a composite type's relid
into the RTE is more dangerous than I thought --- there may be
other code out there doing things similar to set_rel_width().

> Maybe we need to not clear rte->functions, and use that
> instead.

At first I didn't like that idea a bit, and it's still rather ugly,
but it does have two advantages:

* it seems less likely to break anyone's assumptions about the data
structure;

* we'd avoid a purely speculative catcache fetch in
preprocess_function_rtes, which most of the time is a waste of effort
because no subsequent makeWholeRowVar will happen.

We could still clear rte->functions during setrefs.c, so that this
abuse of the data structure is purely local to the planner.

I'll have a go at coding it that way...

			regards, tom lane

  15. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-11T21:51:21Z

    I wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> Maybe we need to not clear rte->functions, and use that
>> instead.

> I'll have a go at coding it that way...

Yeah, that is a better way.  In exchange for a slightly dirtier
data structure, we now have essentially all the relevant code in
makeWholeRowVar: correctness only depends on its different case
branches agreeing, rather than on some not-terribly-similar code
way over in prepjointree.c.

I also took the opportunity to split off the old-branch adjustment
of rewriteHandler.c, so that the HEAD and v15 versions of the main
bug fix patch are nearly the same.

			regards, tom lane

  16. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-12T10:16:13Z

    On Tue, 11 Mar 2025 at 21:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Yeah, that is a better way.  In exchange for a slightly dirtier
> data structure, we now have essentially all the relevant code in
> makeWholeRowVar: correctness only depends on its different case
> branches agreeing, rather than on some not-terribly-similar code
> way over in prepjointree.c.

Agreed. Having all the code in one place makes it easier to see that
it's doing the same thing before and after RTE expansion.

> I also took the opportunity to split off the old-branch adjustment
> of rewriteHandler.c, so that the HEAD and v15 versions of the main
> bug fix patch are nearly the same.

LGTM. I did some more testing and thought about it a little more, and
I can't see any other ways to break it.

Regards,
Dean

  17. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-12T15:49:29Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> LGTM. I did some more testing and thought about it a little more, and
> I can't see any other ways to break it.

Thanks for the careful review and testing!  Pushed after fooling
with the comments a tiny bit more.

			regards, tom lane

  18. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Andres Freund <andres@anarazel.de> — 2025-03-12T22:45:52Z

    Hi,

On 2025-03-12 11:49:29 -0400, Tom Lane wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> > LGTM. I did some more testing and thought about it a little more, and
> > I can't see any other ways to break it.
>
> Thanks for the careful review and testing!  Pushed after fooling
> with the comments a tiny bit more.

This seems to have introduce some breakage for 13-15. E.g. on
sifaka:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sifaka&dt=2025-03-12%2016%3A58%3A51
which has
    'CPPFLAGS' => '-DWRITE_READ_PARSE_PLAN_TREES -DSTRESS_SORT_INT_MIN -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS'

diff -U3 /Users/buildfarm/bf-data/REL_14_STABLE/pgsql.build/src/test/regress/expected/tablespace.out /Users/buildfarm/bf-data/REL_14_STABLE/pgsql.build/src/test/regress/results/tablespace.out
--- /Users/buildfarm/bf-data/REL_14_STABLE/pgsql.build/src/test/regress/expected/tablespace.out	2025-03-12 12:59:22
+++ /Users/buildfarm/bf-data/REL_14_STABLE/pgsql.build/src/test/regress/results/tablespace.out	2025-03-12 12:59:23
@@ -242,6 +242,7 @@

 -- check \\d output
 \\d testschema.foo
+WARNING:  outfuncs/readfuncs failed to produce equal parse tree
               Table "testschema.foo"
  Column |  Type   | Collation | Nullable | Default
 --------+---------+-----------+----------+---------
@@ -320,6 +321,7 @@
 (3 rows)
...

Greetings,

Andres

  19. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-13T00:08:53Z

    Andres Freund <andres@anarazel.de> writes:
> This seems to have introduce some breakage for 13-15. E.g. on
> sifaka:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sifaka&dt=2025-03-12%2016%3A58%3A51
> which has
>     'CPPFLAGS' => '-DWRITE_READ_PARSE_PLAN_TREES -DSTRESS_SORT_INT_MIN -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS'

Ugh.  I supposed that it was okay that 317aba70e etc. didn't touch
outfuncs/readfuncs, but I did not think of
-DWRITE_READ_PARSE_PLAN_TREES.

Perhaps a good hack to deal with that is to make setrefs.c clear
out relid for RTE_SUBQUERY RTEs in those branches.  Then, in the
same way that the rte->function hack doesn't escape the planner,
this one wouldn't either.

			regards, tom lane

  20. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-13T01:13:34Z

    I wrote:
> Ugh.  I supposed that it was okay that 317aba70e etc. didn't touch
> outfuncs/readfuncs, but I did not think of
> -DWRITE_READ_PARSE_PLAN_TREES.

> Perhaps a good hack to deal with that is to make setrefs.c clear
> out relid for RTE_SUBQUERY RTEs in those branches.  Then, in the
> same way that the rte->function hack doesn't escape the planner,
> this one wouldn't either.

Double ugh: that doesn't fix it, because we also do a round of
WRITE_READ_PARSE_PLAN_TREES checks on the rewriter output.
Not sure how to fix this, unless we lobotomize that write/read
check somehow.

			regards, tom lane

  21. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-13T01:39:05Z

    I wrote:
> Double ugh: that doesn't fix it, because we also do a round of
> WRITE_READ_PARSE_PLAN_TREES checks on the rewriter output.
> Not sure how to fix this, unless we lobotomize that write/read
> check somehow.

After some thought, I'm inclined to suggest that we just remove
the post-rewrite check in the affected branches.  This is certainly
not an ideal solution, but unless someone can think of a
fundamentally different way to fix the original bug in these branches,
I don't see a better way.  There are some mitigating points:

* Applying WRITE_READ_PARSE_PLAN_TREES here does not correspond to
any actual system functionality requirement.  We have to be able
to write/read post-parse-analysis trees to store views and rules;
and we have to be able to write/read plan trees to transfer them
to parallel workers.  But there's no real reason why we have to
be able to do it for the purely-transient output of rewriting.

* We aren't going to be changing the outfuncs/readfuncs code
in these branches anymore anyway, for precisely the same reasons
that this patchset can't do that.  So it's not clear what the
test is doing for us.

So that's the best I've got for tonight.  I've verified that the
attached patch for v15 fixes this failure.

			regards, tom lane

  22. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-13T15:10:58Z

    On Thu, 13 Mar 2025 at 01:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> After some thought, I'm inclined to suggest that we just remove
> the post-rewrite check in the affected branches.
>

I don't really have any better ideas. I guess that if we wanted to
keep this check in back branches, we could copy across the subquery
RTE relids onto the new query, like we do for queryIds. But it would
be somewhat messy, because we'd have to do something like walk each
query pulling a list of its RTE_SUBQUERY RTEs, and then walk both
lists, copying relids over. So maybe not worth the effort.

Regards,
Dean

  23. Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-13T16:15:33Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On Thu, 13 Mar 2025 at 01:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> After some thought, I'm inclined to suggest that we just remove
>> the post-rewrite check in the affected branches.

> I don't really have any better ideas. I guess that if we wanted to
> keep this check in back branches, we could copy across the subquery
> RTE relids onto the new query, like we do for queryIds. But it would
> be somewhat messy, because we'd have to do something like walk each
> query pulling a list of its RTE_SUBQUERY RTEs, and then walk both
> lists, copying relids over. So maybe not worth the effort.

Yeah, I don't think it's worth it either -- testing something that's
not actually the behavior of the code-under-test seems pointless.

I've not thought of any better way, so pushed that one to get the
buildfarm back to green.  There's always "git revert" if somebody
has a better idea.

			regards, tom lane