This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit fce2c2905526559626124489102ba07966b63d8c Author: Aegeaner <[email protected]> AuthorDate: Wed Nov 9 10:18:37 2022 +0800 Fix range tables opening's locking issue inside ExecInitModifyTable(). (#14418) As #14417 reported, there will be assertion failures in some cases when executing ModifyTable. This is because we opened all the range table relations in NoLock mode, but sometimes with no lock holding by us. We fixed this by using GpPolicyFetch instead of opening relations since we only need to judge if the range table is a partitioned table. This fix also resolved a GPDB_91_MERGE FIXME left around range table locking code. --- src/backend/executor/nodeModifyTable.c | 13 ++----------- src/test/regress/expected/delete.out | 8 ++++++++ src/test/regress/sql/delete.sql | 7 +++++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 20d6988f9c..1cbc017a81 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -3401,7 +3401,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) PlanRowMark *rc = lfirst_node(PlanRowMark, l); ExecRowMark *erm; ExecAuxRowMark *aerm; - bool isdistributed = false; /* ignore "parent" rowmarks; they are irrelevant at runtime */ if (rc->isParent) @@ -3410,21 +3409,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) /* * Like in preprocess_targetlist, ignore distributed tables. */ - /* - * GPDB_91_MERGE_FIXME: we are largely just ignoring locking altogether. - * Perhaps that's OK as long as we take a full table lock on any UPDATEs - * or DELETEs. But sure doesn't seem right. Can we do better? - */ { RangeTblEntry *rte = rt_fetch(rc->rti, estate->es_plannedstmt->rtable); if (rte->rtekind == RTE_RELATION) { - Relation relation = heap_open(rte->relid, NoLock); - if (GpPolicyIsPartitioned(relation->rd_cdbpolicy)) - isdistributed = true; - heap_close(relation, NoLock); - if (isdistributed) + GpPolicy *policy = GpPolicyFetch(rte->relid); + if (GpPolicyIsPartitioned(policy)) continue; } } diff --git a/src/test/regress/expected/delete.out b/src/test/regress/expected/delete.out index 8446f4226d..56c9168c92 100644 --- a/src/test/regress/expected/delete.out +++ b/src/test/regress/expected/delete.out @@ -30,4 +30,12 @@ SELECT id, a, char_length(b) FROM delete_test; 1 | 10 | (1 row) +-- issue 14417: Range Tables related relations opening's locking issue +CREATE EXTERNAL WEB TABLE dummy(x int) EXECUTE 'touch /tmp/dummy2.csv;cat /tmp/dummy2.csv' ON MASTER FORMAT 'csv'; +CREATE TABLE issue_14417(x int); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'x' as the Greenplum Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +DELETE FROM issue_14417 WHERE x IN (SELECT x FROM dummy); DROP TABLE delete_test; +DROP TABLE issue_14417; +DROP EXTERNAL TABLE dummy; diff --git a/src/test/regress/sql/delete.sql b/src/test/regress/sql/delete.sql index e6a2a39cc6..f0f8dbd783 100644 --- a/src/test/regress/sql/delete.sql +++ b/src/test/regress/sql/delete.sql @@ -22,4 +22,11 @@ DELETE FROM delete_test WHERE a > 25; SELECT id, a, char_length(b) FROM delete_test; +-- issue 14417: Range Tables related relations opening's locking issue +CREATE EXTERNAL WEB TABLE dummy(x int) EXECUTE 'touch /tmp/dummy2.csv;cat /tmp/dummy2.csv' ON MASTER FORMAT 'csv'; +CREATE TABLE issue_14417(x int); +DELETE FROM issue_14417 WHERE x IN (SELECT x FROM dummy); + DROP TABLE delete_test; +DROP TABLE issue_14417; +DROP EXTERNAL TABLE dummy; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
