gpdb: pg_get_expr does not support InvalidOid as reloid
Greenplum version or build
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:
pg_dump- A regression test
- 7 files in documentation
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
- Refcator pg_get_expr() locks related to #11144 — committed to x4m/gpdb by x4m 4 years ago
- Optimize pg_get_expr to avoid unnecessary lock on relation This interface is designed to parse a generic rule expression and rule expressions contain references to relations. Acquiring AccessShareLoc... — committed to JunfengYang/gpdb by deleted user 3 years ago
@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.