Reject wholerow references where ColumnRef is forbidden
Hi Hackers,
Here is a fix to some crashes when using a wholerow reference to sneak
in a ColumnRef where it shouldn't be allowed. This was originally
reported in [0]/messages/by-id/yfers7d2kizq6ockz4nu7e7avq3gj5anpd7ilxkazyvicv4qgy@3cesow2bbhnx by Andres while reviewing FOR PORTION OF, but it is
actually a problem on master, going back at least to v14. You can
reach a crash like so:
-- crashes
ALTER TABLE t ADD COLUMN b int DEFAULT (ROW(t.*)).f1;
-- crashes
CREATE TABLE p1 PARTITION OF p FOR VALUES IN ((ROW(p1.*)).f1);
-- wrongly accepted
CREATE TABLE t (a int, b int DEFAULT (ROW(t.*)).f1);
ALTER TABLE t ALTER COLUMN b SET DEFAULT (ROW(t.*)).f1;
INSERT INTO t (a) VALUES (1); -- crashes here instead
Today, we forbid some EXPR_KINDs in transformColumnRef, but we need to
do that in (or near) ExpandColumnRefStar as well. I factored out the
check to checkColumnRefAllowed to avoid future inconsistencies.
[0]: /messages/by-id/yfers7d2kizq6ockz4nu7e7avq3gj5anpd7ilxkazyvicv4qgy@3cesow2bbhnx
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
Paul A Jungwirth <pj@illuminatedcomputing.com> writes:
Here is a fix to some crashes when using a wholerow reference to sneak
in a ColumnRef where it shouldn't be allowed.
Good catch, but I really don't like this choice:
Actually instead of calling it from ExpandColumnRefStar, I'm calling it
from (one of) its callers, since it isn't always necessary.
That's a recipe for future errors of omission; if someone adds another
call of ExpandColumnRefStar, they might copy the wrong pattern.
Another problem is that I'm not sure that every case of "something.*"
produces Vars; foo().* would not, for example. It might be better to
push the check down to someplace like expandNSItemVars.
regards, tom lane
On Sat, Oct 31, 2026 at 1:24 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Good catch, but I really don't like this choice:
Actually instead of calling it from ExpandColumnRefStar, I'm calling it
from (one of) its callers, since it isn't always necessary.That's a recipe for future errors of omission; if someone adds another
call of ExpandColumnRefStar, they might copy the wrong pattern.Another problem is that I'm not sure that every case of "something.*"
produces Vars; foo().* would not, for example. It might be better to
push the check down to someplace like expandNSItemVars.
Here is a patch that calls checkColumnRefAllowed from expandNSItemVars
instead. It allows foo().* but forbids (ROW(t.*)).f1.
I've included an optional (squashable) second patch that also calls it
from ExpandColumnRefStar, so that even when the table name can't be
resolved, we still get "cannot use column reference" instead of
"missing FROM-clause entry". That seems like the more fundamental
problem, and for defaults & partitions there is no FROM clause anyway.
I've also attached separate patch files for v14/15, since
expandNSItemVars has no pstate. I'm calling checkColumnRefAllowed from
its callers instead to preserve the ABI.
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com