From 756c1d3f5dc59920c5a85eb255945c890c40db0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emel=20=C5=9Eim=C5=9Fek?= Date: Fri, 17 Feb 2023 17:47:03 +0300 Subject: [PATCH] Remove auto_explain workaround in citus explain hook for ALTER TABLE (#6714) When auto_explain module is loaded and configured, EXPLAIN will be implicitly run for all the supported commands. Postgres does not support `EXPLAIN` for `ALTER` command. However, auto_explain will try to `EXPLAIN` other supported commands internally triggered by `ALTER`. For instance, `ALTER TABLE target_table ADD CONSTRAINT fkey_167 FOREIGN KEY (col_1) REFERENCES ref_table(key) ... ` command may trigger a SELECT command in the following form for foreign key validation purpose: `SELECT fk.col_1 FROM ONLY target_table fk LEFT OUTER JOIN ONLY ref_table pk ON ( pk.key OPERATOR(pg_catalog.=) fk.col_1) WHERE pk.key IS NULL AND (fk.col_1 IS NOT NULL) ` For Citus tables, the Citus utility hook should ensure that constraint validation is skipped for shell tables but they are done for shard tables. The reason behind this design choice can be summed up as: - An ALTER TABLE command via coordinator node is run in a distributed transaction. - Citus does not support nested distributed transactions. - A SELECT query on a distributed table (aka shell table) is also run in a distributed transaction. - Therefore, Citus does not support running a SELECT query on a shell table while an ALTER TABLE command is running. With https://github.com/citusdata/citus/commit/eadc88a800cf9c45a11c22a954e27c186156e461 a bug is introduced breaking the skip constraint validation behaviour of Citus. With this change, we see that validation queries on distributed tables are triggered within `ALTER` command adding constraints with validation check. This regression did not cause an issue for regular use cases since the citus executor hook blocks those queries heuristically when there is an ALTER TABLE command in progress. The issue is surfaced as a crash (#6424 Workers, when configured to use auto_explain, crash during distributed transactions.) when auto_explain is enabled. This is due to auto_explain trying to execute the SELECT queries in a nested distributed transaction. Now since the regression with constraint validation is fixed in https://github.com/citusdata/citus/issues/6543, we should be able to remove the workaround. --- src/backend/distributed/planner/multi_explain.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/backend/distributed/planner/multi_explain.c b/src/backend/distributed/planner/multi_explain.c index d7766a913..3d0fe617e 100644 --- a/src/backend/distributed/planner/multi_explain.c +++ b/src/backend/distributed/planner/multi_explain.c @@ -199,20 +199,6 @@ CitusExplainScan(CustomScanState *node, List *ancestors, struct ExplainState *es return; } - /* - * ALTER TABLE statements are not explained by postgres. However ALTER TABLE statements - * may trigger SELECT statements causing explain hook to run. This situation causes a crash in a worker. - * Therefore we will detect if we are explaining a triggered query when we are processing - * an ALTER TABLE statement and stop explain in this situation. - */ - if (AlterTableInProgress()) - { - ExplainPropertyText("Citus Explain Scan", - "Explain for triggered constraint validation queries during ALTER TABLE commands are not supported by Citus", - es); - return; - } - ExplainOpenGroup("Distributed Query", "Distributed Query", true, es); /*