Re: disallow alter individual column if partition key contains wholerow reference - Mailing list pgsql-hackers

>On Tue, Oct 28, 2025 at 11:24?PM Kirill Reshke <reshkekirill@gmail.com> wrote:
>>
>> I can see that 0 - FirstLowInvalidHeapAttributeNumber is an existing
>> coding practise in sources, but so is
>> InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber (sepgsql
>> contrib extension). I'm voting for the second one, even though it is
>> less used.
>>
>
>hi.
>
>now it's:
>+            /*
>+             * If the partition expression contains a whole-row reference,
>+             * then all columns are indirectly associated with that
>+             * expression.
>+             */
>+            if (bms_is_member(InvalidAttrNumber -
>FirstLowInvalidHeapAttributeNumber,
>+                              expr_attrs))
>+            {
>+                if (used_in_expr)
>+                    *used_in_expr = true;
>+                return true;
>+            }
>
>also polished the commit message. below is the commit message:
>--------------------------
>For partition key expressions containing whole-row reference, we cannot rely on
>pg_depend lookups to determine whether an individual column can be altered
>(drop, set data type).
>As noted in the comments for find_expr_references_walker, no dependency entries
>are recorded for whole-row expressions. Therefore whole-row reference check is
>needed in has_partition_attrs.
>
>Partition key expressions contain whole-row reference, it is effectively
>equivalent to all user columns being indirectly associated with that expression.
>Since columns that in a partition key cannot be altered in any way, the same
>restriction should be applied to whole-row reference expressions in the
>partition key.
>--------------------------
>
>
>--
>jian
>https://www.enterprisedb.com/


Hi all,

Thanks for this patch and the surrounding discussions.

I have several points to share for review consideration:
1.Regarding the varattno value used to identify whole-row Vars: I tend to think
InvalidAttrNumber would be a better choice than 0, which aligns with the existing
implementation of makeWholeRowVar().

2.While reviewing this patch, I took a close look at makeWholeRowVar(). Most whole-row Vars
constructed by this function use InvalidAttrNumber for varattno, yet there exists one edge case
where varattno = 1 is used. Given that partition expressions can be highly complex with multi-level nesting,
apart from RTE_RELATION, I am not certain which other RTE types could generate genuine whole-row
references covering all columns of a relation.

I have attempted to reproduce various ways of constructing whole-row Vars to perform negative testing and
verify the robustness of this patch. Although I tried combinations such as custom composite types and
user-defined functions, I only successfully reproduced the scenario for RTE_RELATION so far.

3.This leads to my concern: if we only check for varattno = InvalidAttrNumber in the newly added logic,
can we reliably capture all legitimate whole-row references that refer to every column of a target relation?
There is a risk that this check might be too broad and introduce unnecessary over-restrictions.

4.If the grammar and validation rules for PARTITION BY during table creation strictly restrict whole-row Vars
to only RTE_RELATION type, then this patch is reasonable. As commented in ComputePartitionAttrs():
                /*
                 * transformPartitionSpec() should have already rejected
                 * subqueries, aggregates, window functions, and SRFs, based
                 * on the EXPR_KIND_ for partition expressions.
                 */

5.The newly modified function has_partition_attrs() processes each partition key element following largely
the same logic as RelationBuildPartitionKey() and ComputePartitionAttrs(). Our handling of whole-row Vars
in this patch is consistent with the logic inside ComputePartitionAttrs() for expression branches. The only
difference is that the existing code uses 0 for whole-row detection, while this patch switches to InvalidAttrNumber.
Effectively, both varattno = InvalidAttrNumber and varattno = 0 denote whole-row references to the target relation.

6.To more accurately identify genuine whole-row Vars belonging to the partitioned table itself and avoid misjudgments
from other types of whole-row references or future new whole-row implementations, I would like to propose a stricter
matching rule:
We can collect all Var nodes from the partition expression and validate them with the following constraints to
precisely 
target the whole-row references we intend to guard:
```c
Var->vartype == Relation->reltype (pg_class.reltype) && Var->varno == 1 && Var->varlevelsup == 0
```c


These are my personal thoughts; please feel free to correct any misunderstandings or further discuss these points.


regards,
--
ZizhuanLiu (X-MAN) 
44973863@qq.com