diff --git a/src/postgres/src/backend/executor/nodeModifyTable.c b/src/postgres/src/backend/executor/nodeModifyTable.c index c6a38efc3f20..fdafab8fa2c2 100644 --- a/src/postgres/src/backend/executor/nodeModifyTable.c +++ b/src/postgres/src/backend/executor/nodeModifyTable.c @@ -2400,8 +2400,7 @@ yb_lreplace:; } ModifyTable *plan = (ModifyTable *) context->mtstate->ps.plan; - YbCopySkippableEntities(&estate->yb_skip_entities, - plan->yb_skip_entities); + YbCopySkippableEntities(&estate->yb_skip_entities, plan->yb_skip_entities); /* * If an update is a "single row transaction", then we have already @@ -2430,13 +2429,6 @@ yb_lreplace:; */ *is_pk_updated = YbIsPrimaryKeyUpdated(resultRelationDesc, *cols_marked_for_update); - - /* - * TODO(alex): It probably makes more sense to pass a - * transformed slot instead of a plan slot? Note though - * that it can have tuple materialized already. - */ - if (*is_pk_updated) { YBCExecuteUpdateReplace(resultRelationDesc, context->planSlot, slot, estate); @@ -4589,7 +4581,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) */ mtstate->yb_is_update_optimization_enabled = (node->yb_update_affected_entities != NULL && - operation == CMD_UPDATE && YbIsUpdateOptimizationEnabled()); + YbIsUpdateOptimizationEnabled()); mtstate->yb_is_inplace_index_update_enabled = yb_enable_inplace_index_update; diff --git a/src/postgres/src/backend/executor/ybOptimizeModifyTable.c b/src/postgres/src/backend/executor/ybOptimizeModifyTable.c index 6855db96580d..8d624a1626fa 100644 --- a/src/postgres/src/backend/executor/ybOptimizeModifyTable.c +++ b/src/postgres/src/backend/executor/ybOptimizeModifyTable.c @@ -381,6 +381,11 @@ YbComputeModifiedEntities(ResultRelInfo *resultRelInfo, HeapTuple oldtuple, if (affected_entities == NULL) return NULL; + bool skip_entities_initially_empty PG_USED_FOR_ASSERTS_ONLY = + skip_entities->index_list == NIL && + skip_entities->referenced_fkey_list == NIL && + skip_entities->referencing_fkey_list == NIL; + /* * Clone the update matrix to create a working copy for this tuple. We would * like to preserve a clean copy for subsequent tuples in this query/plan. @@ -442,12 +447,12 @@ YbComputeModifiedEntities(ResultRelInfo *resultRelInfo, HeapTuple oldtuple, /* Mark the primary key as updated */ if (entity->oid == rel->rd_pkindex) { + Assert(skip_entities_initially_empty); /* * In case the primary key is updated, the entire row must be - * deleted and re-inserted. The no update index list is not - * useful in such cases. + * deleted and re-inserted, and all references updated. */ - skip_entities->index_list = NIL; + YbClearSkippableEntities(skip_entities); break; } } diff --git a/src/postgres/src/backend/optimizer/util/ybcplan.c b/src/postgres/src/backend/optimizer/util/ybcplan.c index 48cd761e6083..c400c18deb99 100644 --- a/src/postgres/src/backend/optimizer/util/ybcplan.c +++ b/src/postgres/src/backend/optimizer/util/ybcplan.c @@ -382,6 +382,7 @@ void YbCopySkippableEntities(YbSkippableEntities *dst, const YbSkippableEntities *src) { + YbClearSkippableEntities(dst); if (src == NULL) return; @@ -439,8 +440,11 @@ YbClearSkippableEntities(YbSkippableEntities *skip_entities) return; list_free(skip_entities->index_list); + skip_entities->index_list = NIL; list_free(skip_entities->referencing_fkey_list); + skip_entities->referencing_fkey_list = NIL; list_free(skip_entities->referenced_fkey_list); + skip_entities->referenced_fkey_list = NIL; } static AttrToColIdxMap @@ -804,6 +808,7 @@ YbUpdateComputeIndexColumnReferences(const Relation rel, YbSkippableEntities *skip_entities, int *nentities) { + bool pk_maybe_modified = false; /* * Add the primary key to the head of the entity list so that it gets * evaluated first. @@ -812,7 +817,6 @@ YbUpdateComputeIndexColumnReferences(const Relation rel, { Bitmapset *pkbms = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_PRIMARY_KEY); - bool pk_maybe_modified = false; int attnum = -1; while ((attnum = bms_next_member(pkbms, attnum)) >= 0) { @@ -895,7 +899,7 @@ YbUpdateComputeIndexColumnReferences(const Relation rel, } } - if (!index_maybe_modified) + if (!pk_maybe_modified && !index_maybe_modified) YbAddEntityToSkipList(etype, index_oid, skip_entities); RelationClose(indexDesc); @@ -915,6 +919,9 @@ YbUpdateComputeForeignKeyColumnReferences(const Relation rel, YbSkippableEntityType etype = is_referencing_rel ? SKIP_REFERENCING_FKEY : SKIP_REFERENCED_FKEY; const AttrNumber offset = YBGetFirstLowInvalidAttributeNumber(rel); + Oid pk_oid = RelationGetPrimaryKeyIndex(rel); + bool pk_maybe_modified = + !(OidIsValid(pk_oid) && list_member_oid(skip_entities->index_list, pk_oid)); ListCell *cell; foreach (cell, fkeylist) { @@ -933,7 +940,7 @@ YbUpdateComputeForeignKeyColumnReferences(const Relation rel, } } - if (!fkey_maybe_modified) + if (!pk_maybe_modified && !fkey_maybe_modified) YbAddEntityToSkipList(etype, fkey->conoid, skip_entities); } } diff --git a/src/postgres/src/test/regress/expected/yb_update_optimize_base.out b/src/postgres/src/test/regress/expected/yb_update_optimize_base.out index 00da4d066305..ba275890158c 100644 --- a/src/postgres/src/test/regress/expected/yb_update_optimize_base.out +++ b/src/postgres/src/test/regress/expected/yb_update_optimize_base.out @@ -1375,6 +1375,22 @@ EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET key = 'gparent30' WHERE v Storage Flush Requests: 1 (12 rows) +-- Same query, no modification, no triggers +EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET key = 'gparent30' WHERE value = 'Eddie'; + QUERY PLAN +---------------------------------------------------- + Update on ancestry (actual rows=0 loops=1) + -> Seq Scan on ancestry (actual rows=1 loops=1) + Storage Filter: (value = 'Eddie'::text) + Storage Table Read Requests: 1 + Storage Table Rows Scanned: 10 + Storage Table Write Requests: 1 + Storage Read Requests: 1 + Storage Rows Scanned: 10 + Storage Write Requests: 1 + Storage Flush Requests: 1 +(10 rows) + EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET key = 'gparent3' WHERE key = 'gparent30'; QUERY PLAN -------------------------------------------------------------------------- @@ -1521,3 +1537,80 @@ EXPLAIN (ANALYZE, DIST, COSTS OFF) EXECUTE updateplan1 (1, 1, 2, 1, 1); DEALLOCATE updateplan1; DROP TABLE t_simple; DROP TABLE t_parent; +-- +-- #25075: Test INSERT with UPDATE ON CONFLICT DO UPDATE +-- +DROP TABLE IF EXISTS ioc_table; +NOTICE: table "ioc_table" does not exist, skipping +CREATE TABLE ioc_table (id text, uuid_col uuid, name text, primary key ((id, name), uuid_col)); +CREATE UNIQUE INDEX NONCONCURRENTLY ioc_table_name_id_uidx ON ioc_table (name, id); +CREATE UNIQUE INDEX NONCONCURRENTLY ioc_table_name_uidx ON ioc_table (name); +CREATE UNIQUE INDEX NONCONCURRENTLY ioc_table_uuid_col_uidx ON ioc_table (uuid_col); +CREATE EXTENSION "uuid-ossp"; +-- insert +EXPLAIN (ANALYZE, DIST, COSTS OFF) INSERT INTO ioc_table VALUES ('user-id-1-1', uuid_generate_v4(), 'Test Name 1') + ON CONFLICT (id, name) DO UPDATE SET uuid_col = EXCLUDED.uuid_col; + QUERY PLAN +---------------------------------------------------- + Insert on ioc_table (actual rows=0 loops=1) + Conflict Resolution: UPDATE + Conflict Arbiter Indexes: ioc_table_name_id_uidx + Tuples Inserted: 1 + Conflicting Tuples: 0 + -> Result (actual rows=1 loops=1) + Storage Index Read Requests: 1 + Storage Table Write Requests: 1 + Storage Index Write Requests: 3 + Storage Read Requests: 1 + Storage Rows Scanned: 0 + Storage Write Requests: 4 + Storage Flush Requests: 1 +(13 rows) + +-- conflict: update +EXPLAIN (ANALYZE, DIST, COSTS OFF) INSERT INTO ioc_table VALUES ('user-id-1-1', uuid_generate_v4(), 'Test Name 1') + ON CONFLICT (id, name) DO UPDATE SET uuid_col = EXCLUDED.uuid_col; + QUERY PLAN +---------------------------------------------------- + Insert on ioc_table (actual rows=0 loops=1) + Conflict Resolution: UPDATE + Conflict Arbiter Indexes: ioc_table_name_id_uidx + Tuples Inserted: 0 + Conflicting Tuples: 1 + -> Result (actual rows=1 loops=1) + Storage Table Read Requests: 1 + Storage Table Rows Scanned: 1 + Storage Index Read Requests: 1 + Storage Index Rows Scanned: 1 + Storage Table Write Requests: 2 + Storage Index Write Requests: 4 + Storage Read Requests: 2 + Storage Rows Scanned: 2 + Storage Write Requests: 6 + Storage Flush Requests: 1 +(16 rows) + +-- conflict: update again +EXPLAIN (ANALYZE, DIST, COSTS OFF) INSERT INTO ioc_table VALUES ('user-id-1-1', uuid_generate_v4(), 'Test Name 1') + ON CONFLICT (id, name) DO UPDATE SET uuid_col = EXCLUDED.uuid_col; + QUERY PLAN +---------------------------------------------------- + Insert on ioc_table (actual rows=0 loops=1) + Conflict Resolution: UPDATE + Conflict Arbiter Indexes: ioc_table_name_id_uidx + Tuples Inserted: 0 + Conflicting Tuples: 1 + -> Result (actual rows=1 loops=1) + Storage Table Read Requests: 1 + Storage Table Rows Scanned: 1 + Storage Index Read Requests: 1 + Storage Index Rows Scanned: 1 + Storage Table Write Requests: 2 + Storage Index Write Requests: 4 + Storage Read Requests: 2 + Storage Rows Scanned: 2 + Storage Write Requests: 6 + Storage Flush Requests: 1 +(16 rows) + +DROP TABLE IF EXISTS ioc_table; diff --git a/src/postgres/src/test/regress/sql/yb_update_optimize_base.sql b/src/postgres/src/test/regress/sql/yb_update_optimize_base.sql index b8a86db80d98..226bf150df91 100644 --- a/src/postgres/src/test/regress/sql/yb_update_optimize_base.sql +++ b/src/postgres/src/test/regress/sql/yb_update_optimize_base.sql @@ -312,6 +312,8 @@ EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET parent = 'ggparent23', gp -- Query modifying the key to the row should trigger checks in only the children -- and grandchildren. EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET key = 'gparent30' WHERE value = 'Eddie'; +-- Same query, no modification, no triggers +EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET key = 'gparent30' WHERE value = 'Eddie'; EXPLAIN (ANALYZE, DIST, COSTS OFF) UPDATE ancestry SET key = 'gparent3' WHERE key = 'gparent30'; DROP TABLE ancestry; @@ -387,3 +389,24 @@ DEALLOCATE updateplan1; DROP TABLE t_simple; DROP TABLE t_parent; + +-- +-- #25075: Test INSERT with UPDATE ON CONFLICT DO UPDATE +-- +DROP TABLE IF EXISTS ioc_table; +CREATE TABLE ioc_table (id text, uuid_col uuid, name text, primary key ((id, name), uuid_col)); +CREATE UNIQUE INDEX NONCONCURRENTLY ioc_table_name_id_uidx ON ioc_table (name, id); +CREATE UNIQUE INDEX NONCONCURRENTLY ioc_table_name_uidx ON ioc_table (name); +CREATE UNIQUE INDEX NONCONCURRENTLY ioc_table_uuid_col_uidx ON ioc_table (uuid_col); + +CREATE EXTENSION "uuid-ossp"; +-- insert +EXPLAIN (ANALYZE, DIST, COSTS OFF) INSERT INTO ioc_table VALUES ('user-id-1-1', uuid_generate_v4(), 'Test Name 1') + ON CONFLICT (id, name) DO UPDATE SET uuid_col = EXCLUDED.uuid_col; +-- conflict: update +EXPLAIN (ANALYZE, DIST, COSTS OFF) INSERT INTO ioc_table VALUES ('user-id-1-1', uuid_generate_v4(), 'Test Name 1') + ON CONFLICT (id, name) DO UPDATE SET uuid_col = EXCLUDED.uuid_col; +-- conflict: update again +EXPLAIN (ANALYZE, DIST, COSTS OFF) INSERT INTO ioc_table VALUES ('user-id-1-1', uuid_generate_v4(), 'Test Name 1') + ON CONFLICT (id, name) DO UPDATE SET uuid_col = EXCLUDED.uuid_col; +DROP TABLE IF EXISTS ioc_table;