MIME-Version: 1.0
References: <19099-e05dcfa022fe553d@postgresql.org> <CACJufxF9FcuYe8XOuWLgWK77HCUHpOc6+7+NkktFFNmzw15jKg@mail.gmail.com>
 <CAHewXN=vF5d9O4R3+iUwLqEaP7pb8iYAN_e3vEE_p5sJHofn7w@mail.gmail.com>
 <2960545.1761800903@sss.pgh.pa.us> <CALdSSPi7udsgQg3PUG=Z4+-9pRg8wT3HkDvTgYvtg30xNWQ9OA@mail.gmail.com>
 <CALdSSPi9n2KGzKQn2Egqz3H8Nx0cgnZ8UeB5gk-KVdE3uBCj6Q@mail.gmail.com>
In-Reply-To: <CALdSSPi9n2KGzKQn2Egqz3H8Nx0cgnZ8UeB5gk-KVdE3uBCj6Q@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Thu, 30 Oct 2025 17:29:37 +0900
Message-ID: <CA+HiwqFcejrmS_H8YB-AMB7sujB7wdJXFPdAVfDC6-19FXUjgg@mail.gmail.com>
Subject: Re: BUG #19099: Conditional DELETE from partitioned table with
 non-updatable partition raises internal error
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Tender Wang <tndrwang@gmail.com>, 
	jian he <jian.universality@gmail.com>, exclusion@gmail.com, 
	pgsql-bugs@lists.postgresql.org
Content-Type: multipart/mixed; boundary="000000000000a5e8aa06425c0d5b"
Archived-At: <https://www.postgresql.org/message-id/CA%2BHiwqFcejrmS_H8YB-AMB7sujB7wdJXFPdAVfDC6-19FXUjgg%40mail.gmail.com>
Precedence: bulk

--000000000000a5e8aa06425c0d5b
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, Oct 30, 2025 at 3:44=E2=80=AFPM Kirill Reshke <reshkekirill@gmail.c=
om> wrote:
> On Thu, 30 Oct 2025 at 10:31, I wrote:
> >
> >  I mean, we I believe we need to execute
> > CheckValidResultRel against all partitions in ExecInitModifyTable, at
> > least when no partition pruning has been performed
> >
>
> So, the problem is that we managed to exclude all child relations, and
> only have a single (dummy) root relation as a result of the
> modifyTable plan. Maybe we should populate its target list with
> pseudo-junk columns in create_modifytable_plan ?
>
> For instance, they query does not error-out if we have at least one
> another non-file-fdw partition:
>
> create table p2 partition of pt for values in ( 2) ;
>
> this is because we have this in create_modifytable_plan
>
> ```
> /* Transfer resname/resjunk labeling, too, to keep executor happy */
> apply_tlist_labeling(subplan->targetlist, root->processed_tlist);
> ```
>
> and we successfully found a junk column in the p2 partition.
>
> The problem is, it works iff root->processed_tlist has at least one
> relation which can give us junk columns. Should we add handling for
> corner case here?
> Another option is to remove this 'Transfer resname/resjunk labeling'
> completely and rework planner-executer contracts somehow.

I am not really sure if we should play with the planner code.

I suspect the real issue is that we=E2=80=99re assuming partitioned tables
always need a ctid, which wasn=E2=80=99t true before MERGE started using th=
e
root ResultRelInfo. In fact, the old code already looked wrong -- it=E2=80=
=99s
been requiring a ctid even for partitioned tables where that was never
necessary. We can fix this by only requiring the junk ctid when we
actually operate through the root partitioned table, that is, for
MERGE.  Like the attached.

--=20
Thanks, Amit Langote

--000000000000a5e8aa06425c0d5b
Content-Type: application/octet-stream; 
	name="partitioned-table-ctid-check.diff"
Content-Disposition: attachment; 
	filename="partitioned-table-ctid-check.diff"
Content-Transfer-Encoding: base64
Content-ID: <f_mhd5rxhg0>
X-Attachment-Id: f_mhd5rxhg0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2V4ZWN1dG9yL25vZGVNb2RpZnlUYWJsZS5jIGIvc3Jj
L2JhY2tlbmQvZXhlY3V0b3Ivbm9kZU1vZGlmeVRhYmxlLmMKaW5kZXggNGM1NjQ3YWMzOGEuLjc4
Mzk0YmU2N2VhIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9leGVjdXRvci9ub2RlTW9kaWZ5VGFi
bGUuYworKysgYi9zcmMvYmFja2VuZC9leGVjdXRvci9ub2RlTW9kaWZ5VGFibGUuYwpAQCAtNDM0
OCw4ICs0MzQ4LDEzIEBAIEV4ZWNNb2RpZnlUYWJsZShQbGFuU3RhdGUgKnBzdGF0ZSkKIAkJCQly
ZWxraW5kID09IFJFTEtJTkRfTUFUVklFVyB8fAogCQkJCXJlbGtpbmQgPT0gUkVMS0lORF9QQVJU
SVRJT05FRF9UQUJMRSkKIAkJCXsKLQkJCQkvKiByaV9Sb3dJZEF0dE5vIHJlZmVycyB0byBhIGN0
aWQgYXR0cmlidXRlICovCi0JCQkJQXNzZXJ0KEF0dHJpYnV0ZU51bWJlcklzVmFsaWQocmVzdWx0
UmVsSW5mby0+cmlfUm93SWRBdHRObykpOworCQkJCS8qCisJCQkJICogcmlfUm93SWRBdHRObyBy
ZWZlcnMgdG8gYSBjdGlkIGF0dHJpYnV0ZS4gIEl0IG1heSBiZSBtaXNzaW5nCisJCQkJICogb25s
eSBmb3Igbm9uLU1FUkdFIG9wZXJhdGlvbnMgb24gcGFydGl0aW9uZWQgdGFibGVzLgorCQkJCSAq
LworCQkJCUFzc2VydChBdHRyaWJ1dGVOdW1iZXJJc1ZhbGlkKHJlc3VsdFJlbEluZm8tPnJpX1Jv
d0lkQXR0Tm8pIHx8CisJCQkJCSAgIChyZWxraW5kID09IFJFTEtJTkRfUEFSVElUSU9ORURfVEFC
TEUgJiYKKwkJCQkJCW9wZXJhdGlvbiAhPSBDTURfTUVSR0UpKTsKIAkJCQlkYXR1bSA9IEV4ZWNH
ZXRKdW5rQXR0cmlidXRlKHNsb3QsCiAJCQkJCQkJCQkJCSByZXN1bHRSZWxJbmZvLT5yaV9Sb3dJ
ZEF0dE5vLAogCQkJCQkJCQkJCQkgJmlzTnVsbCk7CkBAIC00ODYzLDcgKzQ4NjgsMTQgQEAgRXhl
Y0luaXRNb2RpZnlUYWJsZShNb2RpZnlUYWJsZSAqbm9kZSwgRVN0YXRlICplc3RhdGUsIGludCBl
ZmxhZ3MpCiAJCQl7CiAJCQkJcmVzdWx0UmVsSW5mby0+cmlfUm93SWRBdHRObyA9CiAJCQkJCUV4
ZWNGaW5kSnVua0F0dHJpYnV0ZUluVGxpc3Qoc3VicGxhbi0+dGFyZ2V0bGlzdCwgImN0aWQiKTsK
LQkJCQlpZiAoIUF0dHJpYnV0ZU51bWJlcklzVmFsaWQocmVzdWx0UmVsSW5mby0+cmlfUm93SWRB
dHRObykpCisKKwkJCQkvKgorCQkJCSAqIEZvciBoZWFwIHJlbGF0aW9ucywgYSBjdGlkIGp1bmsg
YXR0cmlidXRlIG11c3QgYmUgcHJlc2VudC4KKwkJCQkgKiBGb3IgcGFydGl0aW9uZWQgdGFibGVz
LCByZXF1aXJlIGl0IG9ubHkgZm9yIE1FUkdFOyBhbGxvdyBpdCB0byBiZQorCQkJCSAqIG1pc3Np
bmcgZm9yIG5vbi1NRVJHRSBvcGVyYXRpb25zLgorCQkJCSAqLworCQkJCWlmICghQXR0cmlidXRl
TnVtYmVySXNWYWxpZChyZXN1bHRSZWxJbmZvLT5yaV9Sb3dJZEF0dE5vKSAmJgorCQkJCQkocmVs
a2luZCAhPSBSRUxLSU5EX1BBUlRJVElPTkVEX1RBQkxFIHx8IG9wZXJhdGlvbiA9PSBDTURfTUVS
R0UpKQogCQkJCQllbG9nKEVSUk9SLCAiY291bGQgbm90IGZpbmQganVuayBjdGlkIGNvbHVtbiIp
OwogCQkJfQogCQkJZWxzZSBpZiAocmVsa2luZCA9PSBSRUxLSU5EX0ZPUkVJR05fVEFCTEUpCg==
--000000000000a5e8aa06425c0d5b--