From 1cb17c7aeb5716895d4247c8a984ab84000d6244 Mon Sep 17 00:00:00 2001 From: reshke Date: Mon, 19 Aug 2024 00:14:49 +0500 Subject: [PATCH 1/2] Remove redundant param from 'ivm_visible_in_prestate' call. At first glance, i did get the neediness of this parameter. The visibility of tuple on QE does not depend on segment id, only on exported snapshot. So, this is clearly unneeded. But... why IVM test are not failing on this? This got me some more digging in CBDB internals, I actually think all this code is simply dead code for Cloudberry. Prestate visibility in IVM is something that matters when multiple relation are modified via SINGLE statement. I don't think we can even do this in Cloudberry: 1) Multiple write CTE are not possible, because there could be only one writer gang. Ref: https://github.com/cloudberrydb/cloudberrydb/commit/bfcb78829d50e31a49ce18d36d9c3af9b8a2ffe3 2) After statement are also prohibited, I get 'Triggers for statements are not yet supported.' error with them. Are there any other way for multiple relation modification in PostgreSQL that works for GP/CBDB? And, yes, even when this will (if even) be supported, gpseg id is anyway redundant. --- src/backend/commands/matview.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 9d9b66ead95..542d122f413 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -2175,7 +2175,7 @@ get_prestate_rte(RangeTblEntry *rte, MV_TriggerTable *table, initStringInfo(&str); appendStringInfo(&str, "SELECT t.* FROM %s t" - " WHERE pg_catalog.ivm_visible_in_prestate(t.tableoid, t.ctid,%d::pg_catalog.oid, t.gp_segment_id)", + " WHERE pg_catalog.ivm_visible_in_prestate(t.tableoid, t.ctid, %d::pg_catalog.oid)", relname, matviewid); /* @@ -3857,4 +3857,4 @@ ivm_set_ts_persitent_name(TriggerData *trigdata, Oid relid, Oid mvid) oldctx = MemoryContextSwitchTo(TopMemoryContext); SetTransitionTableName(relid, cmd, mvid); MemoryContextSwitchTo(oldctx); -} \ No newline at end of file +} From a9e1719a14f6557e5c78c1df97e420f5f8ebaf2c Mon Sep 17 00:00:00 2001 From: reshke Date: Mon, 19 Aug 2024 06:54:49 +0000 Subject: [PATCH 2/2] Fix tests & add fixme --- src/backend/commands/matview.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 542d122f413..6484d35744a 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -2093,6 +2093,11 @@ ivm_visible_in_prestate(PG_FUNCTION_ARGS) Oid tableoid = PG_GETARG_OID(0); ItemPointer itemPtr = PG_GETARG_ITEMPOINTER(1); Oid matviewOid = PG_GETARG_OID(2); + /* + CBDB_PG_15_FIXME: Here, we do not use the 4th argument of this function. + Either justify its existence by using it, or remove the 4th argument from + the function definition (catalog change). + */ ListCell *lc; bool result = true; bool found = false; @@ -2175,7 +2180,7 @@ get_prestate_rte(RangeTblEntry *rte, MV_TriggerTable *table, initStringInfo(&str); appendStringInfo(&str, "SELECT t.* FROM %s t" - " WHERE pg_catalog.ivm_visible_in_prestate(t.tableoid, t.ctid, %d::pg_catalog.oid)", + " WHERE pg_catalog.ivm_visible_in_prestate(t.tableoid, t.ctid, %d::pg_catalog.oid, t.gp_segment_id)", relname, matviewid); /* @@ -2189,6 +2194,8 @@ get_prestate_rte(RangeTblEntry *rte, MV_TriggerTable *table, tuplestore_get_sharedname(tuplestore)); } + elogif(Debug_print_ivm, INFO, "IVM execute prestate visibilty chek new %s", str.data); + /* Get a subquery representing pre-state of the table */ raw = (RawStmt*)linitial(raw_parser(str.data, RAW_PARSE_DEFAULT)); subquery = transformStmt(pstate, raw->stmt);