PartitionKeyData->partattrs, refactor some 0 to InvalidAttrNumber
hi.
if PartitionKeyData->partattrs is 0, then it means that partition key is
expression, else it's column reference.
we can change from
if (key->partattrs[i] == 0)
to
if (key->partattrs[i] == InvalidAttrNumber)
the reason I can think of is to improve grepability.
numeric value 0 is too common to reliably locate relevant code.
what do you think?
Attachments:
Hi,
On Fri, Nov 14, 2025 at 11:47:35PM +0800, jian he wrote:
hi.
if PartitionKeyData->partattrs is 0, then it means that partition key is
expression, else it's column reference.we can change from
if (key->partattrs[i] == 0)
to
yes, I think that makes sense to not compare an AttrNumber type to 0, but...
if (key->partattrs[i] == InvalidAttrNumber)
I think that we should make use of AttributeNumberIsValid() instead. Same
spirit as we've done in a2b02293bc6.
FWIW, I'm looking at the Oid/OidIsValid() cases currently (writing a coccinelle
script).
I can look at the AttrNumber/AttributeNumberIsValid() cases after (that should be
as easy as changing Oid to AttrNumber and OidIsValid() to AttributeNumberIsValid()
in the coccinelle script), unless you want to take care of the AttrNumber case?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Sat, Nov 15, 2025 at 12:59 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
Hi,
On Fri, Nov 14, 2025 at 11:47:35PM +0800, jian he wrote:
hi.
if PartitionKeyData->partattrs is 0, then it means that partition key is
expression, else it's column reference.we can change from
if (key->partattrs[i] == 0)
toyes, I think that makes sense to not compare an AttrNumber type to 0, but...
if (key->partattrs[i] == InvalidAttrNumber)
I think that we should make use of AttributeNumberIsValid() instead. Same
spirit as we've done in a2b02293bc6.
hi.
AttributeNumberIsValid makes more sense.
Attached changes on pg_partitioned_table, pg_index are based on
AttributeNumberIsValid.
FWIW, I'm looking at the Oid/OidIsValid() cases currently (writing a coccinelle
script).I can look at the AttrNumber/AttributeNumberIsValid() cases after (that should be
as easy as changing Oid to AttrNumber and OidIsValid() to AttributeNumberIsValid()
in the coccinelle script), unless you want to take care of the AttrNumber case?
Currently, I am only interested in pg_partitioned_table.partattrs,
pg_index.indkey.