public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v7 3/7] Propagate changes to indisclustered to child/parents 3+ messages / 3 participants [nested] [flat]
* [PATCH v7 3/7] Propagate changes to indisclustered to child/parents @ 2020-10-07 03:11 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-10-07 03:11 UTC (permalink / raw) --- src/backend/commands/cluster.c | 109 ++++++++++++++++---------- src/backend/commands/indexcmds.c | 2 + src/test/regress/expected/cluster.out | 46 +++++++++++ src/test/regress/sql/cluster.sql | 11 +++ 4 files changed, 125 insertions(+), 43 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 9b6673867c..78c2c2ba72 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -74,6 +74,7 @@ static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose); static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); +static void set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index); static List *get_tables_to_cluster(MemoryContext cluster_context); static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid); @@ -510,66 +511,88 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD index_close(OldIndex, NoLock); } +/* + * Helper for mark_index_clustered + * Mark a single index as clustered or not. + * pg_index is passed by caller to avoid repeatedly re-opening it. + */ +static void +set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index) +{ + HeapTuple indexTuple; + Form_pg_index indexForm; + + indexTuple = SearchSysCacheCopy1(INDEXRELID, + ObjectIdGetDatum(indexOid)); + if (!HeapTupleIsValid(indexTuple)) + elog(ERROR, "cache lookup failed for index %u", indexOid); + indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + + /* this was checked earlier, but let's be real sure */ + if (isclustered && !indexForm->indisvalid) + elog(ERROR, "cannot cluster on invalid index %u", indexOid); + + indexForm->indisclustered = isclustered; + CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); + heap_freetuple(indexTuple); +} + /* * mark_index_clustered: mark the specified index as the one clustered on * - * With indexOid == InvalidOid, will mark all indexes of rel not-clustered. + * With indexOid == InvalidOid, mark all indexes of rel not-clustered. + * Otherwise, mark children of the clustered index as clustered, and parents of + * other indexes as unclustered. + * We wish to maintain the following properties: + * 1) Only one index on a relation can be marked clustered at once + * 2) If a partitioned index is clustered, then all its children must be + * clustered. */ void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) { - HeapTuple indexTuple; - Form_pg_index indexForm; - Relation pg_index; - ListCell *index; - - /* - * If the index is already marked clustered, no need to do anything. - */ - if (OidIsValid(indexOid)) - { - if (get_index_isclustered(indexOid)) - return; - } + ListCell *lc, *lc2; + List *indexes; + Relation pg_index = table_open(IndexRelationId, RowExclusiveLock); + List *inh = find_all_inheritors(RelationGetRelid(rel), ShareRowExclusiveLock, NULL); /* * Check each index of the relation and set/clear the bit as needed. + * Iterate over the relation's children rather than the index's children + * since we need to unset cluster for indexes on intermediate children, + * too. */ - pg_index = table_open(IndexRelationId, RowExclusiveLock); - - foreach(index, RelationGetIndexList(rel)) + foreach(lc, inh) { - Oid thisIndexOid = lfirst_oid(index); - - indexTuple = SearchSysCacheCopy1(INDEXRELID, - ObjectIdGetDatum(thisIndexOid)); - if (!HeapTupleIsValid(indexTuple)) - elog(ERROR, "cache lookup failed for index %u", thisIndexOid); - indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + Oid inhrelid = lfirst_oid(lc); + Relation thisrel = table_open(inhrelid, ShareRowExclusiveLock); - /* - * Unset the bit if set. We know it's wrong because we checked this - * earlier. - */ - if (indexForm->indisclustered) + indexes = RelationGetIndexList(thisrel); + foreach (lc2, indexes) { - indexForm->indisclustered = false; - CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); - } - else if (thisIndexOid == indexOid) - { - /* this was checked earlier, but let's be real sure */ - if (!indexForm->indisvalid) - elog(ERROR, "cannot cluster on invalid index %u", indexOid); - indexForm->indisclustered = true; - CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); - } + bool isclustered; + Oid thisIndexOid = lfirst_oid(lc2); + List *parentoids = get_rel_relispartition(thisIndexOid) ? + get_partition_ancestors(thisIndexOid) : NIL; - InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0, - InvalidOid, is_internal); + /* + * A child of the clustered index must be set clustered; + * indexes which are not children of the clustered index are + * set unclustered + */ + isclustered = (thisIndexOid == indexOid) || + list_member_oid(parentoids, indexOid); + Assert(OidIsValid(indexOid) || !isclustered); + set_indisclustered(thisIndexOid, isclustered, pg_index); + + InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0, + InvalidOid, is_internal); + } - heap_freetuple(indexTuple); + list_free(indexes); + table_close(thisrel, ShareRowExclusiveLock); } + list_free(inh); table_close(pg_index, RowExclusiveLock); } diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 127ba7835d..4ca1ffbfa4 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -25,6 +25,7 @@ #include "catalog/catalog.h" #include "catalog/index.h" #include "catalog/indexing.h" +#include "catalog/partition.h" #include "catalog/pg_am.h" #include "catalog/pg_constraint.h" #include "catalog/pg_inherits.h" @@ -32,6 +33,7 @@ #include "catalog/pg_opfamily.h" #include "catalog/pg_tablespace.h" #include "catalog/pg_type.h" +#include "commands/cluster.h" #include "commands/comment.h" #include "commands/dbcommands.h" #include "commands/defrem.h" diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index c74cfa88cc..1d436dfaae 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -495,6 +495,52 @@ Indexes: "clstrpart_idx" btree (a) CLUSTER Number of partitions: 3 (Use \d+ to list them.) +-- Test that it recurses to grandchildren: +\d clstrpart33 + Table "public.clstrpart33" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart3 DEFAULT +Indexes: + "clstrpart33_a_idx" btree (a) CLUSTER + +ALTER TABLE clstrpart SET WITHOUT CLUSTER; +\d clstrpart33 + Table "public.clstrpart33" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart3 DEFAULT +Indexes: + "clstrpart33_a_idx" btree (a) + +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +\d clstrpart33 + Table "public.clstrpart33" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart3 DEFAULT +Indexes: + "clstrpart33_a_idx" btree (a) CLUSTER + +-- Check that only one child is marked clustered after marking clustered on a different parent +CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a); +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2; +\d clstrpart1 + Partitioned table "public.clstrpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart FOR VALUES FROM (1) TO (10) +Partition key: RANGE (a) +Indexes: + "clstrpart1_a_idx" btree (a) + "clstrpart1_idx_2" btree (a) CLUSTER +Number of partitions: 2 (Use \d+ to list them.) + -- Test CLUSTER with external tuplesorting create table clstr_4 as select * from tenk1; create index cluster_sort on clstr_4 (hundred, thousand, tenthous); diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index 9bcc77695c..0ded2be1ca 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -220,6 +220,17 @@ CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitio CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf \d clstrpart +-- Test that it recurses to grandchildren: +\d clstrpart33 +ALTER TABLE clstrpart SET WITHOUT CLUSTER; +\d clstrpart33 +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +\d clstrpart33 +-- Check that only one child is marked clustered after marking clustered on a different parent +CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a); +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2; +\d clstrpart1 -- Test CLUSTER with external tuplesorting -- 2.17.0 --AA9g+nFNFPYNJKiL Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0004-Invalidate-parent-indexes.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Output affected rows in EXPLAIN @ 2023-09-07 14:57 Damir Belyalov <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Damir Belyalov @ 2023-09-07 14:57 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: pgsql-hackers; Andres Freund <[email protected]>; [email protected]; Daniel Gustafsson <[email protected]> > This creates a bug, not fixes one. It's intentional that "insert into a" > is shown as returning zero rows, because that's what it did. If you'd > written "insert ... returning", you'd have gotten a different result: > Maybe I didn't understand you correctly, but I didn't touch the number of affected rows in EXPLAIN output. It's just a simple patch that adds 1 row after using commands: EXPLAIN INSERT, EXPLAIN UPDATE, EXPLAIN DELETE. It was done because the commands INSERT/UPDATE/DELETE return one row after execution: "UPDATE 7" or "INSERT 0 4". EXPLAIN (ANALYZE) INSERT/UPDATE/DELETE does the same thing as these commands, but doesn't output this row. So I added it. Patch is fixed. There is no row "EXPLAIN" in queries like: postgres=# explain (analyze) select * from t; QUERY PLAN ----------------------------------------------------------------------------------------------- Seq Scan on t (cost=0.00..35.50 rows=2550 width=4) (actual time=0.064..0.075 rows=5 loops=1) Planning Time: 1.639 ms Execution Time: 0.215 ms (3 rows) EXPLAIN What is about queries EXPLAIN INSERT/UPDATE/DELETE without ANALYZE? Now it is outputting a row with 0 affected (inserted) rows at the end: "INSERT 0 0", "UPDATE 0". Example: explain update a set n = 2; QUERY PLAN ------------------------------------------------------------ Update on a (cost=0.00..35.50 rows=0 width=0) -> Seq Scan on a (cost=0.00..35.50 rows=2550 width=10) (2 rows) UPDATE 0 Regards, Damir Belyalov Postgres Professional Attachments: [text/x-patch] 0002-Output-affected-rows-in-EXPLAIN.patch (9.5K, ../../CALH1Lgvpj7cMKLn4Xu+jJ_ra4TLocCpELCHqt1UYzYWQqmi_Rw@mail.gmail.com/3-0002-Output-affected-rows-in-EXPLAIN.patch) download | inline diff: From c6cbc6fa9ddf24f29bc19ff115224dd76e351db1 Mon Sep 17 00:00:00 2001 From: Damir Belyalov <[email protected]> Date: Tue, 5 Sep 2023 15:04:01 +0300 Subject: [PATCH 1/2] Output affected rows in EXPLAIN. --- src/backend/commands/explain.c | 10 +++++++++- src/backend/tcop/cmdtag.c | 2 +- src/backend/tcop/pquery.c | 8 +++++++- src/backend/tcop/utility.c | 27 ++++++++++++++++++++++++++- src/bin/psql/common.c | 5 +++-- src/include/commands/explain.h | 3 ++- src/include/tcop/cmdtag.h | 1 + src/include/tcop/cmdtaglist.h | 3 +++ src/interfaces/libpq/fe-exec.c | 4 +++- 9 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 8570b14f62..453e545ba5 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -162,7 +162,7 @@ static void escape_yaml(StringInfo buf, const char *str); */ void ExplainQuery(ParseState *pstate, ExplainStmt *stmt, - ParamListInfo params, DestReceiver *dest) + ParamListInfo params, DestReceiver *dest, uint64 *processed) { ExplainState *es = NewExplainState(); TupOutputState *tstate; @@ -173,6 +173,9 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, bool timing_set = false; bool summary_set = false; + if (processed) + *processed = 0; + /* Parse options list. */ foreach(lc, stmt->options) { @@ -311,6 +314,9 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, end_tup_output(tstate); pfree(es->str->data); + + if (processed) + *processed = es->es_processed; } /* @@ -649,6 +655,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, */ INSTR_TIME_SET_CURRENT(starttime); + es->es_processed += queryDesc->estate->es_processed; + ExecutorEnd(queryDesc); FreeQueryDesc(queryDesc); diff --git a/src/backend/tcop/cmdtag.c b/src/backend/tcop/cmdtag.c index 4bd713a0b4..9e6fdbd8af 100644 --- a/src/backend/tcop/cmdtag.c +++ b/src/backend/tcop/cmdtag.c @@ -146,7 +146,7 @@ BuildQueryCompletionString(char *buff, const QueryCompletion *qc, */ if (command_tag_display_rowcount(tag) && !nameonly) { - if (tag == CMDTAG_INSERT) + if (tag == CMDTAG_INSERT || tag == CMDTAG_EXPLAIN_INSERT) { *bufp++ = ' '; *bufp++ = '0'; diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 5565f200c3..ba0b33cc67 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -775,7 +775,13 @@ PortalRun(Portal portal, long count, bool isTopLevel, bool run_once, if (qc && portal->qc.commandTag != CMDTAG_UNKNOWN) { CopyQueryCompletion(qc, &portal->qc); - qc->nprocessed = nprocessed; + if (portal->qc.commandTag == CMDTAG_EXPLAIN || + portal->qc.commandTag == CMDTAG_EXPLAIN_INSERT || + portal->qc.commandTag == CMDTAG_EXPLAIN_UPDATE || + portal->qc.commandTag == CMDTAG_EXPLAIN_DELETE) + qc->nprocessed = portal->qc.nprocessed; + else + qc->nprocessed = nprocessed; } /* Mark portal not active */ diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index e3ccf6c7f7..8975d046f9 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -867,7 +867,32 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_ExplainStmt: - ExplainQuery(pstate, (ExplainStmt *) parsetree, params, dest); + { + Query *query; + uint64 processed; + int explainTag; + + ExplainQuery(pstate, (ExplainStmt *) parsetree, params, dest, &processed); + + query = castNode(Query, ((ExplainStmt *) parsetree)->query); + switch (query->commandType) + { + case CMD_INSERT: + explainTag = CMDTAG_EXPLAIN_INSERT; + break; + case CMD_UPDATE: + explainTag = CMDTAG_EXPLAIN_UPDATE; + break; + case CMD_DELETE: + explainTag = CMDTAG_EXPLAIN_DELETE; + break; + default: + explainTag = CMDTAG_EXPLAIN; + break; + } + if (qc) + SetQueryCompletion(qc, explainTag, processed); + } break; case T_AlterSystemStmt: diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index ede197bebe..a66d9127c5 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -987,8 +987,9 @@ PrintQueryResult(PGresult *result, bool last, if (last || pset.show_all_results) { cmdstatus = PQcmdStatus(result); - if (strncmp(cmdstatus, "INSERT", 6) == 0 || - strncmp(cmdstatus, "UPDATE", 6) == 0 || + if (strncmp(cmdstatus, "EXPLAIN", 7) == 0 || + strncmp(cmdstatus, "INSERT", 6) == 0 || + strncmp(cmdstatus, "UPDATE", 6) == 0 || strncmp(cmdstatus, "DELETE", 6) == 0) PrintQueryStatus(result, printStatusFout); } diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h index 3d3e632a0c..21fe5f7555 100644 --- a/src/include/commands/explain.h +++ b/src/include/commands/explain.h @@ -60,6 +60,7 @@ typedef struct ExplainState bool hide_workers; /* set if we find an invisible Gather */ /* state related to the current plan node */ ExplainWorkersState *workers_state; /* needed if parallel plan */ + uint64 es_processed; /* sum of queryDesc->estate->es_processed */ } ExplainState; /* Hook for plugins to get control in ExplainOneQuery() */ @@ -78,7 +79,7 @@ extern PGDLLIMPORT explain_get_index_name_hook_type explain_get_index_name_hook; extern void ExplainQuery(ParseState *pstate, ExplainStmt *stmt, - ParamListInfo params, DestReceiver *dest); + ParamListInfo params, DestReceiver *dest, uint64 *processed); extern ExplainState *NewExplainState(void); diff --git a/src/include/tcop/cmdtag.h b/src/include/tcop/cmdtag.h index 1e7514dcff..49f7ea85e7 100644 --- a/src/include/tcop/cmdtag.h +++ b/src/include/tcop/cmdtag.h @@ -30,6 +30,7 @@ typedef enum CommandTag typedef struct QueryCompletion { CommandTag commandTag; + CommandTag explainCommandTag; uint64 nprocessed; } QueryCompletion; diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h index e738ac1c09..fdc570a304 100644 --- a/src/include/tcop/cmdtaglist.h +++ b/src/include/tcop/cmdtaglist.h @@ -178,6 +178,9 @@ PG_CMDTAG(CMDTAG_DROP_USER_MAPPING, "DROP USER MAPPING", true, false, false) PG_CMDTAG(CMDTAG_DROP_VIEW, "DROP VIEW", true, false, false) PG_CMDTAG(CMDTAG_EXECUTE, "EXECUTE", false, false, false) PG_CMDTAG(CMDTAG_EXPLAIN, "EXPLAIN", false, false, false) +PG_CMDTAG(CMDTAG_EXPLAIN_INSERT, "INSERT", false, false, true) +PG_CMDTAG(CMDTAG_EXPLAIN_UPDATE, "UPDATE", false, false, true) +PG_CMDTAG(CMDTAG_EXPLAIN_DELETE, "DELETE", false, false, true) PG_CMDTAG(CMDTAG_FETCH, "FETCH", false, false, true) PG_CMDTAG(CMDTAG_GRANT, "GRANT", true, false, false) PG_CMDTAG(CMDTAG_GRANT_ROLE, "GRANT ROLE", false, false, false) diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index c6d80ec396..b85e9b8e04 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -3741,7 +3741,9 @@ PQcmdTuples(PGresult *res) if (!res) return ""; - if (strncmp(res->cmdStatus, "INSERT ", 7) == 0) + if (strncmp(res->cmdStatus, "EXPLAIN ", 8) == 0) + p = res->cmdStatus + 8; + else if (strncmp(res->cmdStatus, "INSERT ", 7) == 0) { p = res->cmdStatus + 7; /* INSERT: skip oid and space */ -- 2.34.1 From eceaa19b847b18ee3346b0d2fc526e69557b71bd Mon Sep 17 00:00:00 2001 From: Damir Belyalov <[email protected]> Date: Thu, 7 Sep 2023 17:27:21 +0300 Subject: [PATCH 2/2] v2 Output affected rows in EXPLAIN. --- src/bin/psql/common.c | 5 ++--- src/include/tcop/cmdtag.h | 1 - src/interfaces/libpq/fe-exec.c | 6 ++---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index a66d9127c5..1f39dcef4b 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -983,12 +983,11 @@ PrintQueryResult(PGresult *result, bool last, else success = true; - /* if it's INSERT/UPDATE/DELETE RETURNING, also print status */ + /* if it's EXPLAIN/INSERT/UPDATE/DELETE RETURNING, also print status */ if (last || pset.show_all_results) { cmdstatus = PQcmdStatus(result); - if (strncmp(cmdstatus, "EXPLAIN", 7) == 0 || - strncmp(cmdstatus, "INSERT", 6) == 0 || + if (strncmp(cmdstatus, "INSERT", 6) == 0 || strncmp(cmdstatus, "UPDATE", 6) == 0 || strncmp(cmdstatus, "DELETE", 6) == 0) PrintQueryStatus(result, printStatusFout); diff --git a/src/include/tcop/cmdtag.h b/src/include/tcop/cmdtag.h index 49f7ea85e7..1e7514dcff 100644 --- a/src/include/tcop/cmdtag.h +++ b/src/include/tcop/cmdtag.h @@ -30,7 +30,6 @@ typedef enum CommandTag typedef struct QueryCompletion { CommandTag commandTag; - CommandTag explainCommandTag; uint64 nprocessed; } QueryCompletion; diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index b85e9b8e04..40ea2d7ac2 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -3726,7 +3726,7 @@ PQoidValue(const PGresult *res) /* * PQcmdTuples - - * If the last command was INSERT/UPDATE/DELETE/MERGE/MOVE/FETCH/COPY, + * If the last command was EXPLAIN/INSERT/UPDATE/DELETE/MERGE/MOVE/FETCH/COPY, * return a string containing the number of inserted/affected tuples. * If not, return "". * @@ -3741,9 +3741,7 @@ PQcmdTuples(PGresult *res) if (!res) return ""; - if (strncmp(res->cmdStatus, "EXPLAIN ", 8) == 0) - p = res->cmdStatus + 8; - else if (strncmp(res->cmdStatus, "INSERT ", 7) == 0) + if (strncmp(res->cmdStatus, "INSERT ", 7) == 0) { p = res->cmdStatus + 7; /* INSERT: skip oid and space */ -- 2.34.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Output affected rows in EXPLAIN @ 2023-11-15 07:15 [email protected] parent: Damir Belyalov <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: [email protected] @ 2023-11-15 07:15 UTC (permalink / raw) To: Damir Belyalov <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers; Andres Freund <[email protected]>; [email protected]; Daniel Gustafsson <[email protected]> Hi hackers, Indeed, I think it is a little confusing that when executing EXPLAIN(ANALYZE), even though an update is actually occurring, the commandtag of the update result is not returned. However, the manual also describes the information that will be affected when EXPLAIN (ANALYZE) is executed as important information. https://www.postgresql.org/docs/current/sql-explain.html Also, in most cases, users who use EXPLAIN(ANALYZE) only want an execution plan of a statement. If command tags are not required, this can be controlled using the QUIET variable, but command tags other than EXPLAIN will also be omitted, increasing the scope of the effect. We can check the number of updated rows from execute plan, I think there is no need to return the command tag when EXPLAIN(ANALYZE) is executed by default. ## patch and QUIET=off(default) postgres=# explain (analyze) insert into a values (1); QUERY PLAN ------------------------------------------------------------------------------------------ Insert on a (cost=0.00..0.01 rows=0 width=0) (actual time=0.227..0.228 rows=0 loops=1) -> Result (cost=0.00..0.01 rows=1 width=4) (actual time=0.013..0.015 rows=1 loops=1) Planning Time: 0.152 ms Execution Time: 0.480 ms (4 rows) INSERT 0 1 ## patch and QUIET=on(psql work quietly) 'INSERT 0 1' is omitted both 'explain(analyze) and 'INSERT'. postgres=# \set QUIET on postgres=# explain (analyze) insert into a values (1); QUERY PLAN ------------------------------------------------------------------------------------------ Insert on a (cost=0.00..0.01 rows=0 width=0) (actual time=0.058..0.059 rows=0 loops=1) -> Result (cost=0.00..0.01 rows=1 width=4) (actual time=0.004..0.005 rows=1 loops=1) Planning Time: 0.059 ms Execution Time: 0.117 ms (4 rows) postgres=# insert into a values (1); postgres=# Best Regards, Keisuke Kuroda NTT COMWARE On 2023-09-07 23:57, Damir Belyalov wrote: >> This creates a bug, not fixes one. It's intentional that "insert >> into a" >> is shown as returning zero rows, because that's what it did. If >> you'd >> written "insert ... returning", you'd have gotten a different >> result: > > Maybe I didn't understand you correctly, but I didn't touch the number > of affected rows in EXPLAIN output. > It's just a simple patch that adds 1 row after using commands: EXPLAIN > INSERT, EXPLAIN UPDATE, EXPLAIN DELETE. > It was done because the commands INSERT/UPDATE/DELETE return one row > after execution: "UPDATE 7" or "INSERT 0 4". > EXPLAIN (ANALYZE) INSERT/UPDATE/DELETE does the same thing as these > commands, but doesn't output this row. So I added it. > > Patch is fixed. There is no row "EXPLAIN" in queries like: > > postgres=# explain (analyze) select * from t; > QUERY PLAN > ----------------------------------------------------------------------------------------------- > Seq Scan on t (cost=0.00..35.50 rows=2550 width=4) (actual > time=0.064..0.075 rows=5 loops=1) > Planning Time: 1.639 ms > Execution Time: 0.215 ms > (3 rows) > > EXPLAIN > > What is about queries EXPLAIN INSERT/UPDATE/DELETE without ANALYZE? > Now it is outputting a row with 0 affected (inserted) rows at the end: > "INSERT 0 0", "UPDATE 0". Example: > explain update a set n = 2; > QUERY PLAN > ------------------------------------------------------------ > Update on a (cost=0.00..35.50 rows=0 width=0) > -> Seq Scan on a (cost=0.00..35.50 rows=2550 width=10) > (2 rows) > > UPDATE 0 > > Regards, > Damir Belyalov > Postgres Professional ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-11-15 07:15 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-10-07 03:11 [PATCH v7 3/7] Propagate changes to indisclustered to child/parents Justin Pryzby <[email protected]> 2023-09-07 14:57 Re: Output affected rows in EXPLAIN Damir Belyalov <[email protected]> 2023-11-15 07:15 ` Re: Output affected rows in EXPLAIN [email protected]
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox