gpdb: pg_get_expr does not support InvalidOid as reloid

Greenplum version or build

Current 7.X Current 6.X

Expected behavior

pg_get_expr must support InvalidOid passed as its second argument for Var-free expressions. This is described in its comment, and in PostgreSQL documentation:

if no Vars are expected, zero is sufficient

Actual behavior

Due to presence of this code (6.X), NULL is always returned when the second argument of pg_get_expr is InvalidOid.

The code in question is still present in 7.X, although pg_catalog.pg_partitions has been dropped in PostgreSQL 12 merge.

Moreover, pg_partitions is still referred to in:

Discussion

What is the purpose of the lock acquired? Why a call of pg_get_expr in pg_partitions https://github.com/greenplum-db/gpdb/blob/826699c7b931ae5ba16c0362bcf4397a3737fb0b/src/backend/catalog/system_views.sql#L826 cannot be replaced by a call with the second parameter set to InvalidOid: pg_get_expr(pr1.parlistvalues, InvalidOid)? As far as I understand, pg_partitions passes only Const nodes to pg_get_expr, and they do not require a valid OID.

As far as I can see, the change was introduced by @pengzhout originally in https://github.com/greenplum-db/gpdb/pull/7478.

I have read a report on the related bug, but it is quite brief. Is it possible to clarify it?

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 17 (14 by maintainers)

Commits related to this issue

Most upvoted comments

@leskin-in it’s not regression tests. It’s part of upgrdae scripts. I’m hesitant to remove anything from code paths without test coverage. I think ripping out pg_partitions (or re-implementing it) is unrelated big chunk of work. This problem affects pg_dump too.