From 61d11ec2d4c9a2534d60d2aa167be7b7e54c8c4d Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Fri, 20 Dec 2024 17:59:09 +0300 Subject: [PATCH] PG17 Compatibility - Fix HideCitusDependentObjects function (#7796) There is a crash when running vanilla tests because of the `citus.hide_citus_dependent_objects` GUC. We turn on this GUC only for the pg vanilla tests. This GUC runs the following function `HideCitusDependentObjectsOnQueriesOfPgMetaTables`. This function doesn't take into account the new `mergeJoinCondition`. I rewrote the function such that it checks for merge join conditions as well. Relevant PG commit: https://github.com/postgres/postgres/commit/0294df2f1 The crash could be reproduced locally like the following: ```SQL SET citus.hide_citus_dependent_objects TO on; CREATE OR REPLACE FUNCTION pg_catalog.is_citus_depended_object(oid,oid) RETURNS bool LANGUAGE C AS 'citus', $$is_citus_depended_object$$; -- try a system catalog MERGE INTO pg_class c USING (SELECT 'pg_depend'::regclass AS oid) AS j ON j.oid = c.oid WHEN MATCHED THEN UPDATE SET reltuples = reltuples + 1 RETURNING j.oid; CREATE VIEW classv AS SELECT * FROM pg_class; MERGE INTO classv c USING pg_namespace n ON n.oid = c.relnamespace WHEN MATCHED AND c.oid = 'pg_depend'::regclass THEN UPDATE SET reltuples = reltuples - 1 RETURNING c.oid; -- crash happens here ``` --- .../distributed/utils/citus_depended_object.c | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/utils/citus_depended_object.c b/src/backend/distributed/utils/citus_depended_object.c index 3babf76f0..bc14490b5 100644 --- a/src/backend/distributed/utils/citus_depended_object.c +++ b/src/backend/distributed/utils/citus_depended_object.c @@ -243,12 +243,24 @@ HideCitusDependentObjectsOnQueriesOfPgMetaTables(Node *node, void *context) if (OidIsValid(metaTableOid)) { + bool mergeJoinCondition = false; +#if PG_VERSION_NUM >= PG_VERSION_17 + + /* + * In Postgres 17, the query tree has a specific field for the merge condition. + * So we shouldn't modify the jointree, but rather the mergeJoinCondition here + * Relevant PG17 commit: 0294df2f1 + */ + mergeJoinCondition = query->mergeJoinCondition; +#endif + /* * We found a valid pg meta class in query, * so we assert below conditions. */ - Assert(query->jointree != NULL); - Assert(query->jointree->fromlist != NULL); + Assert(mergeJoinCondition || + (query->jointree != NULL && + query->jointree->fromlist != NULL)); Node *citusDependentObjExpr = CreateCitusDependentObjectExpr(varno, metaTableOid); @@ -257,8 +269,18 @@ HideCitusDependentObjectsOnQueriesOfPgMetaTables(Node *node, void *context) * We do not use security quals because a postgres vanilla test fails * with a change of order for its result. */ - query->jointree->quals = make_and_qual( - query->jointree->quals, citusDependentObjExpr); + if (!mergeJoinCondition) + { + query->jointree->quals = make_and_qual( + query->jointree->quals, citusDependentObjExpr); + } + else + { +#if PG_VERSION_NUM >= PG_VERSION_17 + query->mergeJoinCondition = make_and_qual( + query->mergeJoinCondition, citusDependentObjExpr); +#endif + } } MemoryContextSwitchTo(originalContext);