Euler Taveira <eu...@eulerto.com> wrote: > I started reviewing this patch. It was in my radar to review it but I didn't > have spare time until now.
Thanks! > + <refnamediv> > + <refname>REPACK</refname> > + <refpurpose>cluster a table according to an index</refpurpose> > + </refnamediv> > > This description is not accurate because the index is optional. It means if > the > index is not specified, it is a "rewrite" instead of a "cluster". One > suggestion is to use "rewrite" because it says nothing about the tuple order. > A > "rewrite a table" or "rewrite a table to reclaim space" are good candidates. > On > the other hand, the command is called "repack" and it should be a strong > candidate for the verb in this description. (I'm surprised that repack is not > a > recent term [1]). It seems a natural choice. This reveals that I used the documentation of CLUSTER for "inspiration" :-) I prefer "rewrite" because it can help users which don't know what REPACK means in this context. > +<synopsis> > +REPACK [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ > <replaceable class="parameter">table_name</replaceable> > [ USING INDEX<replaceable class="parameter">index_name</replaceable> ] ] ok, fixed > You missed a space after INDEX. > > + <para> > > + Because the planner records statistics about the ordering of tables, it > is > + advisable to > > + run <link linkend="sql-analyze"><command>ANALYZE</command></link> on the > > + newly repacked table. Otherwise, the planner might make poor choices of > > + query plans. > > + </para> > > If we decide for another term (other than "repacked") then it should reflect > here and in some other parts ("repacking" is also used) too. I think it's ok to say "repack" as long as we explained it above. The documentation of CLUSTER command also uses the verb "cluster". > + <para> > > + When an index scan or a sequential scan without sort is used, a > temporary > + copy of the table is created that contains the table data in the index > > + order. > > That's true for CLUSTER but not for REPACK. Index is optional. ok, removed the mention of index. > + Prints a progress report as each table is clustered > > + at <literal>INFO</literal> level. > > s/clustered/repacked/? right > + Repacking a partitioned table repacks each of its partitions. If an > index > + is specified, each partition is clustered using the partition of that > > + index. <command>REPACK</command> on a partitioned table cannot be > executed > + inside a transaction block. > > Ditto. fixed > + <para> > > + Cluster the table <literal>employees</literal> on the basis of its > > + index <literal>employees_ind</literal>: > > +<programlisting> > > +REPACK employees USING INDEX employees_ind; > > +</programlisting> > > + </para> > > It sounds strange to use "Repack" in the other examples but this one it says > "Cluster". Let's use the same terminology. It's explained above on the page that if index is specified, it's clustering. I changed it to "Repack", but added a note that this is effectively clustering. > + > + <warning> > + <para> > + The <command>FULL</command> parameter is deprecated in favor of > + <xref linkend="sql-repack"/>. > + </para> > + </warning> > + > > The warnings, notes, and tips are usually placed *after* the description. You probably mean the subsecions "Notes on Clustering" and "Notes on Resources". I moved them into the "Notes" section. > --- a/src/backend/access/heap/heapam_handler.c > +++ b/src/backend/access/heap/heapam_handler.c > @@ -741,13 +741,13 @@ heapam_relation_copy_for_cluster(Relation OldHeap, > Relation NewHeap, > > Is it worth to rename table_relation_copy_for_cluster() and > heapam_relation_copy_for_cluster() to replace cluster with repack? I had thought about it and concluded that it'd make the patch too invasive. Note that the CLUSTER still uses these functions. We can do the renaming when removing the CLUSTER command someday. > + SELECT > + S.pid AS pid, > + S.datid AS datid, > + D.datname AS datname, > + S.relid AS relid, > + CASE S.param1 WHEN 1 THEN 'REPACK' > + END AS command, > > Do you really need command? IIUC REPACK is the only command that will used by > this view. There is no need to differentiate commands here. REPACK is a regular command, so why shouldn't it have its view? Just like CLUSTER has one (pg_stat_progress_cluster). > + * > + * 'cmd' indicates which commands is being executed. REPACK should be the > only > + * caller of this function in the future. > > command. Not sure I understand this comment. > + * > + * REPACK does not set indisclustered. XXX Not sure I understand the > + * comment above: how can an attribute be set "only in the current > + * database"? > */ > > pg_index is a local catalog. To be consistent while clustering a shared > catalog, it should set indisclustered in all existent databases because in > each > pg_index table there is a tuple for the referred index. As the comment says it > is not possible. Yes, Alvaro already explained this to me [1] :-) > - if (OidIsValid(indexOid) && OldHeap->rd_rel->relisshared) > + if (cmd == CLUSTER_COMMAND_CLUSTER && OldHeap->rd_rel->relisshared) > ereport(ERROR, > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > - errmsg("cannot cluster a shared catalog"))); > + errmsg("cannot %s a shared catalog", cmd_str))); > > I'm confused about this change. Why is it required? > > If it prints this message only for CLUSTER command, you don't need to have a > generic message. This kind of message is not good for translation. If you need > multiple verbs here, I advise you to break it into multiple messages. Good point, I didn't think about translation. Fixed. > - { > - if (OidIsValid(indexOid)) > - ereport(ERROR, > - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > - errmsg("cannot cluster temporary tables of other > sessions"))); > - else > - ereport(ERROR, > - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > - errmsg("cannot vacuum temporary tables of other > sessions"))); > - } > + ereport(ERROR, > + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > + errmsg("cannot %s temporary tables of other sessions", > + cmd_str))); > > Ditto. Fixed. > - CheckTableNotInUse(OldHeap, OidIsValid(indexOid) ? "CLUSTER" : "VACUUM"); > + CheckTableNotInUse(OldHeap, asc_toupper(cmd_str, strlen(cmd_str))); > > If the idea is to remove CLUSTER and VACUUM from this routine in the future, I > wouldn't include formatting.h just for asc_toupper(). Instead, I would use an > if condition. I think it will be easy to remove this code path when the time > comes. Fixed. > - errmsg("cannot cluster on index \"%s\" because access method > does not support clustering", > - RelationGetRelationName(OldIndex)))); > + errmsg("cannot %s on index \"%s\" because access method does > not support clustering", > + cmd_str, RelationGetRelationName(OldIndex)))); > > Ditto. I don't think check_index_is_clusterable() should be changed. The > action > is "cluster" independently of the command. You can keep "cluster" until we > completely remove CLUSTER command and then we can replace this term with > "repack". It also applies to cluster_is_permitted_for_relation(). > > - errmsg("cannot cluster on partial index \"%s\"", > + errmsg("cannot %s on partial index \"%s\"", > + cmd_str, > RelationGetRelationName(OldIndex)))); > > Ditto. > > - errmsg("cannot cluster on invalid index \"%s\"", > - RelationGetRelationName(OldIndex)))); > + errmsg("cannot %s on invalid index \"%s\"", > + cmd_str, RelationGetRelationName(OldIndex)))); > > Ditto. > > - (errmsg("clustering \"%s.%s\" using index scan on \"%s\"", > + (errmsg("%sing \"%s.%s\" using index scan on \"%s\"", > + cmd_str, > nspname, > RelationGetRelationName(OldHeap), > RelationGetRelationName(OldIndex)))); > > This is bad for translation. Use complete sentences. > > - (errmsg("clustering \"%s.%s\" using sequential scan and sort", > + (errmsg("%sing \"%s.%s\" using sequential scan and sort", > + cmd_str, > nspname, > RelationGetRelationName(OldHeap)))); > > Ditto. > > - (errmsg("vacuuming \"%s.%s\"", > + (errmsg("%sing \"%s.%s\"", > + cmd_str, > nspname, > RelationGetRelationName(OldHeap)))); > > Ditto. fixed > /* > - * Given an index on a partitioned table, return a list of RelToCluster for > + * Like get_tables_to_cluster(), but do not care about indexes. > + */ > Since the goal is to remove CLUSTER in the future, provide a comment that > doesn't mention routines that will certainly be removed. Hence, there is no > need to fix them in the future. It'd be almost duplicate of the header comment of get_tables_to_cluster() and I don't like duplication. Let's do that at removal time. > + /* > + * Get all indexes that have indisclustered set and that the current user > + * has the appropriate privileges for. > + */ > > This comment is not true. Fixed. > ereport(WARNING, > - (errmsg("permission denied to cluster \"%s\", skipping it", > + (errmsg("permission denied to %s \"%s\", skipping it", > + CLUSTER_COMMAND_STR(cmd), > get_rel_name(relid)))); > > Fix for translation. Fixed. > + if (stmt->relation != NULL) > + { > + rel = process_single_relation(stmt->relation, stmt->indexname, > + CLUSTER_COMMAND_REPACK, ¶ms, > + &indexOid); > + if (rel == NULL) > + return; > + } > > This code path is confusing. It took me some time (after reading > process_single_relation() that could have a better name) to understand it. I > don't have a good suggestion but it should have at least one comment > explaining > what the purpose is. ok, added the comment that I lost when moving the code from cluster() to process_single_relation(). > +/* > + * REPACK a single relation. > + * > + * Return NULL if done, relation reference if the caller needs to process it > + * (because the relation is partitioned). > + */ > > This comment should be expanded. As I said in the previous hunk, there isn't > sufficient information to understand how process_single_relation() works. This function only contains code that I moved from cluster_rel(). The header comment is and additional information. I tried to rephrase it a bit anyway. > + | REPACK > + { > + RepackStmt *n = makeNode(RepackStmt); > + > + n->relation = NULL; > + n->indexname = NULL; > + n->params = NIL; > + $$ = (Node *) n; > + } > + > + | REPACK '(' utility_option_list ')' > + { > + RepackStmt *n = makeNode(RepackStmt); > + > + n->relation = NULL; > + n->indexname = NULL; > + n->params = $3; > + $$ = (Node *) n; > + } Maybe, will think about it. > I'm wondering if there is an easy way to avoid these rules. > > PROGRESS_COMMAND_VACUUM, > PROGRESS_COMMAND_ANALYZE, > PROGRESS_COMMAND_CLUSTER, > + PROGRESS_COMMAND_REPACK, > PROGRESS_COMMAND_CREATE_INDEX, > PROGRESS_COMMAND_BASEBACKUP, > PROGRESS_COMMAND_COPY, > > It is just a matter of style but I have the habit to include new stuff at the > end. Yes, it seems so. Fixed. > +-- Yet another code path: REPACK w/o index. > +REPACK clstr_tst USING INDEX clstr_tst_c; > +-- Verify that inheritance link still works > > You forgot to remove the USING INDEX here. Good catch. I removed the test because w/o index the output order can be unstable. (Whether index is used or not should not affect the catalog changes related to inheritance or FKs anyway.) > I'm still review the other patches (that is basically the implementation of > CONCURRENTLY) and to avoid a long review, I'm sending the 0001 review. Anyway, > 0001 is independent of the other patches and should be applied separately. Attached is a new version of 0001. As for the other patches, please skip the parts > 0004 - most of this code will be removed [2]. I'll try to post the next version of the patch set next week. (Regarding next reviews, please try to keep hunk headers in the text.) [1] https://www.postgresql.org/message-id/202503031807.dnacvpgnjkz7@alvherre.pgsql [2] https://www.postgresql.org/message-id/13028.1743762516%40localhost -- Antonin Houska Web: https://www.cybertec-postgresql.com
>From af419c5d5f56429581a263f12f70d12144a5a0e9 Mon Sep 17 00:00:00 2001 From: Antonin Houska <a...@cybertec.at> Date: Fri, 4 Apr 2025 18:30:42 +0200 Subject: [PATCH] Add REPACK command. The existing CLUSTER command as well as VACUUM with the FULL option both reclaim unused space by rewriting table. Now that we want to enhance this functionality (in particular, by adding a new option CONCURRENTLY), we should enhance both commands because they are both implemented by the same function (cluster.c:cluster_rel). However, adding the same option to two different commands is not very user-friendly. Therefore it was decided to create a new command and to declare both CLUSTER command and the FULL option of VACUUM deprecated. Future enhancements to this rewriting code will only affect the new command. Like CLUSTER, the REPACK command reorders the table according to the specified index. Unlike CLUSTER, REPACK does not require the index: if only table is specified, the command acts as VACUUM FULL. As we don't want to remove CLUSTER and VACUUM FULL yet, there are three callers of the cluster_rel() function now: REPACK, CLUSTER and VACUUM FULL. When we need to distinguish who is calling this function (mostly for logging, but also for progress reporting), we can no longer use the OID of the clustering index: both REPACK and VACUUM FULL can pass InvalidOid. Therefore, this patch introduces a new enumeration type ClusterCommand, and adds an argument of this type to the cluster_rel() function and to all the functions that need to distinguish the caller. Like CLUSTER and VACUUM FULL, the REPACK COMMAND without arguments processes all the tables on which the current user has the MAINTAIN privilege. A new view pg_stat_progress_repack view is added to monitor the progress of REPACK. Currently it displays the same information as pg_stat_progress_cluster (except that column names might differ), but it'll also display the status of the REPACK CONCURRENTLY command in the future, so the view definitions will eventually diverge. Regarding user documentation, the patch moves the information on clustering from cluster.sgml to the new file repack.sgml. cluster.sgml now contains a link that points to the related section of repack.sgml. A note on deprecation and a link to repack.sgml are added to both cluster.sgml and vacuum.sgml. --- doc/src/sgml/monitoring.sgml | 230 +++++++++++++ doc/src/sgml/ref/allfiles.sgml | 1 + doc/src/sgml/ref/cluster.sgml | 79 +---- doc/src/sgml/ref/repack.sgml | 256 ++++++++++++++ doc/src/sgml/ref/vacuum.sgml | 8 + doc/src/sgml/reference.sgml | 1 + src/backend/access/heap/heapam_handler.c | 32 +- src/backend/catalog/index.c | 2 +- src/backend/catalog/system_views.sql | 27 ++ src/backend/commands/cluster.c | 416 +++++++++++++++++------ src/backend/commands/vacuum.c | 3 +- src/backend/parser/gram.y | 63 +++- src/backend/tcop/utility.c | 9 + src/backend/utils/adt/pgstatfuncs.c | 2 + src/bin/psql/tab-complete.in.c | 31 +- src/include/commands/cluster.h | 19 +- src/include/commands/progress.h | 60 +++- src/include/nodes/parsenodes.h | 13 + src/include/parser/kwlist.h | 1 + src/include/tcop/cmdtaglist.h | 1 + src/include/utils/backend_progress.h | 1 + src/test/regress/expected/cluster.out | 123 +++++++ src/test/regress/expected/rules.out | 27 ++ src/test/regress/sql/cluster.sql | 59 ++++ src/tools/pgindent/typedefs.list | 2 + 25 files changed, 1259 insertions(+), 207 deletions(-) create mode 100644 doc/src/sgml/ref/repack.sgml diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index a6d67d2fbaa..0a6229c391a 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -400,6 +400,14 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser </entry> </row> + <row> + <entry><structname>pg_stat_progress_repack</structname><indexterm><primary>pg_stat_progress_repack</primary></indexterm></entry> + <entry>One row for each backend running + <command>REPACK</command>, showing current progress. See + <xref linkend="repack-progress-reporting"/>. + </entry> + </row> + <row> <entry><structname>pg_stat_progress_basebackup</structname><indexterm><primary>pg_stat_progress_basebackup</primary></indexterm></entry> <entry>One row for each WAL sender process streaming a base backup, @@ -5940,6 +5948,228 @@ FROM pg_stat_get_backend_idset() AS backendid; </table> </sect2> + <sect2 id="repack-progress-reporting"> + <title>REPACK Progress Reporting</title> + + <indexterm> + <primary>pg_stat_progress_repack</primary> + </indexterm> + + <para> + Whenever <command>REPACK</command> is running, + the <structname>pg_stat_progress_repack</structname> view will contain a + row for each backend that is currently running the command. The tables + below describe the information that will be reported and provide + information about how to interpret it. + </para> + + <table id="pg-stat-progress-repack-view" xreflabel="pg_stat_progress_repack"> + <title><structname>pg_stat_progress_repack</structname> View</title> + <tgroup cols="1"> + <thead> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + Column Type + </para> + <para> + Description + </para></entry> + </row> + </thead> + + <tbody> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>pid</structfield> <type>integer</type> + </para> + <para> + Process ID of backend. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>datid</structfield> <type>oid</type> + </para> + <para> + OID of the database to which this backend is connected. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>datname</structfield> <type>name</type> + </para> + <para> + Name of the database to which this backend is connected. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>relid</structfield> <type>oid</type> + </para> + <para> + OID of the table being repacked. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>command</structfield> <type>text</type> + </para> + <para> + The command that is running. Currently, the only value + is <literal>REPACK</literal>. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>phase</structfield> <type>text</type> + </para> + <para> + Current processing phase. See <xref linkend="repack-phases"/>. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>repack_index_relid</structfield> <type>oid</type> + </para> + <para> + If the table is being scanned using an index, this is the OID of the + index being used; otherwise, it is zero. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>heap_tuples_scanned</structfield> <type>bigint</type> + </para> + <para> + Number of heap tuples scanned. + This counter only advances when the phase is + <literal>seq scanning heap</literal>, + <literal>index scanning heap</literal> + or <literal>writing new heap</literal>. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>heap_tuples_written</structfield> <type>bigint</type> + </para> + <para> + Number of heap tuples written. + This counter only advances when the phase is + <literal>seq scanning heap</literal>, + <literal>index scanning heap</literal> + or <literal>writing new heap</literal>. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>heap_blks_total</structfield> <type>bigint</type> + </para> + <para> + Total number of heap blocks in the table. This number is reported + as of the beginning of <literal>seq scanning heap</literal>. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>heap_blks_scanned</structfield> <type>bigint</type> + </para> + <para> + Number of heap blocks scanned. This counter only advances when the + phase is <literal>seq scanning heap</literal>. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>index_rebuild_count</structfield> <type>bigint</type> + </para> + <para> + Number of indexes rebuilt. This counter only advances when the phase + is <literal>rebuilding index</literal>. + </para></entry> + </row> + </tbody> + </tgroup> + </table> + + <table id="repack-phases"> + <title>REPACK Phases</title> + <tgroup cols="2"> + <colspec colname="col1" colwidth="1*"/> + <colspec colname="col2" colwidth="2*"/> + <thead> + <row> + <entry>Phase</entry> + <entry>Description</entry> + </row> + </thead> + + <tbody> + <row> + <entry><literal>initializing</literal></entry> + <entry> + The command is preparing to begin scanning the heap. This phase is + expected to be very brief. + </entry> + </row> + <row> + <entry><literal>seq scanning heap</literal></entry> + <entry> + The command is currently scanning the table using a sequential scan. + </entry> + </row> + <row> + <entry><literal>index scanning heap</literal></entry> + <entry> + <command>REPACK</command> is currently scanning the table using an index scan. + </entry> + </row> + <row> + <entry><literal>sorting tuples</literal></entry> + <entry> + <command>REPACK</command> is currently sorting tuples. + </entry> + </row> + <row> + <entry><literal>writing new heap</literal></entry> + <entry> + <command>REPACK</command> is currently writing the new heap. + </entry> + </row> + <row> + <entry><literal>swapping relation files</literal></entry> + <entry> + The command is currently swapping newly-built files into place. + </entry> + </row> + <row> + <entry><literal>rebuilding index</literal></entry> + <entry> + The command is currently rebuilding an index. + </entry> + </row> + <row> + <entry><literal>performing final cleanup</literal></entry> + <entry> + The command is performing final cleanup. When this phase is + completed, <command>REPACK</command> will end. + </entry> + </row> + </tbody> + </tgroup> + </table> + </sect2> + <sect2 id="copy-progress-reporting"> <title>COPY Progress Reporting</title> diff --git a/doc/src/sgml/ref/allfiles.sgml b/doc/src/sgml/ref/allfiles.sgml index f5be638867a..c0ef654fcb4 100644 --- a/doc/src/sgml/ref/allfiles.sgml +++ b/doc/src/sgml/ref/allfiles.sgml @@ -167,6 +167,7 @@ Complete list of usable sgml source files in this directory. <!ENTITY refreshMaterializedView SYSTEM "refresh_materialized_view.sgml"> <!ENTITY reindex SYSTEM "reindex.sgml"> <!ENTITY releaseSavepoint SYSTEM "release_savepoint.sgml"> +<!ENTITY repack SYSTEM "repack.sgml"> <!ENTITY reset SYSTEM "reset.sgml"> <!ENTITY revoke SYSTEM "revoke.sgml"> <!ENTITY rollback SYSTEM "rollback.sgml"> diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 8811f169ea0..54bb2362c84 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -42,17 +42,23 @@ CLUSTER [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <r <replaceable class="parameter">table_name</replaceable>. </para> - <para> - When a table is clustered, it is physically reordered - based on the index information. Clustering is a one-time operation: - when the table is subsequently updated, the changes are - not clustered. That is, no attempt is made to store new or - updated rows according to their index order. (If one wishes, one can - periodically recluster by issuing the command again. Also, setting - the table's <literal>fillfactor</literal> storage parameter to less than - 100% can aid in preserving cluster ordering during updates, since updated - rows are kept on the same page if enough space is available there.) - </para> + <warning> + <para> + The <command>CLUSTER</command> command is deprecated in favor of + <xref linkend="sql-repack"/>. + </para> + </warning> + + <note> + <para> + <xref linkend="sql-repack-notes-on-clustering"/> explain how clustering + works, whether it is initiated by <command>CLUSTER</command> or + by <command>REPACK</command>. The notable difference between the two is + that <command>REPACK</command> does not remember the index used last + time. Thus if you don't specify an index, <command>REPACK</command> + rewrites the table but does not try to cluster it. + </para> + </note> <para> When a table is clustered, <productname>PostgreSQL</productname> @@ -136,63 +142,12 @@ CLUSTER [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <r on the table. </para> - <para> - In cases where you are accessing single rows randomly - within a table, the actual order of the data in the - table is unimportant. However, if you tend to access some - data more than others, and there is an index that groups - them together, you will benefit from using <command>CLUSTER</command>. - If you are requesting a range of indexed values from a table, or a - single indexed value that has multiple rows that match, - <command>CLUSTER</command> will help because once the index identifies the - table page for the first row that matches, all other rows - that match are probably already on the same table page, - and so you save disk accesses and speed up the query. - </para> - - <para> - <command>CLUSTER</command> can re-sort the table using either an index scan - on the specified index, or (if the index is a b-tree) a sequential - scan followed by sorting. It will attempt to choose the method that - will be faster, based on planner cost parameters and available statistical - information. - </para> - <para> While <command>CLUSTER</command> is running, the <xref linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog, pg_temp</literal>. </para> - <para> - When an index scan is used, a temporary copy of the table is created that - contains the table data in the index order. Temporary copies of each - index on the table are created as well. Therefore, you need free space on - disk at least equal to the sum of the table size and the index sizes. - </para> - - <para> - When a sequential scan and sort is used, a temporary sort file is - also created, so that the peak temporary space requirement is as much - as double the table size, plus the index sizes. This method is often - faster than the index scan method, but if the disk space requirement is - intolerable, you can disable this choice by temporarily setting <xref - linkend="guc-enable-sort"/> to <literal>off</literal>. - </para> - - <para> - It is advisable to set <xref linkend="guc-maintenance-work-mem"/> to - a reasonably large value (but not more than the amount of RAM you can - dedicate to the <command>CLUSTER</command> operation) before clustering. - </para> - - <para> - Because the planner records statistics about the ordering of - tables, it is advisable to run <link linkend="sql-analyze"><command>ANALYZE</command></link> - on the newly clustered table. - Otherwise, the planner might make poor choices of query plans. - </para> - <para> Because <command>CLUSTER</command> remembers which indexes are clustered, one can cluster the tables one wants clustered manually the first time, diff --git a/doc/src/sgml/ref/repack.sgml b/doc/src/sgml/ref/repack.sgml new file mode 100644 index 00000000000..2fcbd75106f --- /dev/null +++ b/doc/src/sgml/ref/repack.sgml @@ -0,0 +1,256 @@ +<!-- +doc/src/sgml/ref/repack.sgml +PostgreSQL documentation +--> + +<refentry id="sql-repack"> + <indexterm zone="sql-repack"> + <primary>REPACK</primary> + </indexterm> + + <refmeta> + <refentrytitle>REPACK</refentrytitle> + <manvolnum>7</manvolnum> + <refmiscinfo>SQL - Language Statements</refmiscinfo> + </refmeta> + + <refnamediv> + <refname>REPACK</refname> + <refpurpose>rewrite a table to reclaim disk space</refpurpose> + </refnamediv> + + <refsynopsisdiv> +<synopsis> +REPACK [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] +[ <replaceable class="parameter">table_name</replaceable> [ USING INDEX +<replaceable class="parameter">index_name</replaceable> ] ] + +<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> + + VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] +</synopsis> + </refsynopsisdiv> + + <refsect1> + <title>Description</title> + + <para> + <command>REPACK</command> reclaims storage occupied by dead + tuples. Unlike <command>VACUUM</command>, it does so by rewriting the + entire contents of the table specified + by <replaceable class="parameter">table_name</replaceable> into a new disk + file with no extra space (except for the space guaranteed by + the <literal>fillfactor</literal> storage parameter), allowing unused space + to be returned to the operating system. + </para> + + <para> + Without + a <replaceable class="parameter">table_name</replaceable>, <command>REPACK</command> + processes every table and materialized view in the current database that + the current user has the <literal>MAINTAIN</literal> privilege on. This + form of <command>REPACK</command> cannot be executed inside a transaction + block. + </para> + + <para> + If <replaceable class="parameter">index_name</replaceable> is specified, + the table is clustered by this index. Please see the notes on clustering + below. + </para> + + <para> + When a table is being repacked, an <literal>ACCESS EXCLUSIVE</literal> lock + is acquired on it. This prevents any other database operations (both reads + and writes) from operating on the table until the <command>REPACK</command> + is finished. + </para> + + </refsect1> + + <refsect1> + <title>Parameters</title> + + <variablelist> + <varlistentry> + <term><replaceable class="parameter">table_name</replaceable></term> + <listitem> + <para> + The name (possibly schema-qualified) of a table. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">index_name</replaceable></term> + <listitem> + <para> + The name of an index. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>VERBOSE</literal></term> + <listitem> + <para> + Prints a progress report as each table is repacked + at <literal>INFO</literal> level. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">boolean</replaceable></term> + <listitem> + <para> + Specifies whether the selected option should be turned on or off. + You can write <literal>TRUE</literal>, <literal>ON</literal>, or + <literal>1</literal> to enable the option, and <literal>FALSE</literal>, + <literal>OFF</literal>, or <literal>0</literal> to disable it. The + <replaceable class="parameter">boolean</replaceable> value can also + be omitted, in which case <literal>TRUE</literal> is assumed. + </para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> + + <refsect1> + <title>Notes</title> + + <para> + To repack a table, one must have the <literal>MAINTAIN</literal> privilege + on the table. + </para> + + <para> + While <command>REPACK</command> is running, the <xref + linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog, + pg_temp</literal>. + </para> + + <para> + Each backend running <command>REPACK</command> will report its progress + in the <structname>pg_stat_progress_repack</structname> view. See + <xref linkend="repack-progress-reporting"/> for details. + </para> + + <para> + Repacking a partitioned table repacks each of its partitions. If an index + is specified, each partition is repacked using the partition of that + index. <command>REPACK</command> on a partitioned table cannot be executed + inside a transaction block. + </para> + + <refsect2 id="sql-repack-notes-on-clustering" xreflabel="Notes on Clustering"> + <title>Notes on Clustering</title> + + <para> + When a table is clustered, it is physically reordered based on the index + information. Clustering is a one-time operation: when the table is + subsequently updated, the changes are not clustered. That is, no attempt + is made to store new or updated rows according to their index order. (If + one wishes, one can periodically recluster by issuing the command again. + Also, setting the table's <literal>fillfactor</literal> storage parameter + to less than 100% can aid in preserving cluster ordering during updates, + since updated rows are kept on the same page if enough space is available + there.) + </para> + + <para> + In cases where you are accessing single rows randomly within a table, the + actual order of the data in the table is unimportant. However, if you tend + to access some data more than others, and there is an index that groups + them together, you will benefit from using <command>REPACK</command>. If + you are requesting a range of indexed values from a table, or a single + indexed value that has multiple rows that match, + <command>REPACK</command> will help because once the index identifies the + table page for the first row that matches, all other rows that match are + probably already on the same table page, and so you save disk accesses and + speed up the query. + </para> + + <para> + <command>REPACK</command> can re-sort the table using either an index scan + on the specified index (if the index is a b-tree), or a sequential scan + followed by sorting. It will attempt to choose the method that will be + faster, based on planner cost parameters and available statistical + information. + </para> + + <para> + Because the planner records statistics about the ordering of tables, it is + advisable to + run <link linkend="sql-analyze"><command>ANALYZE</command></link> on the + newly repacked table. Otherwise, the planner might make poor choices of + query plans. + </para> + </refsect2> + + <refsect2 id="sql-repack-notes-on-resources" xreflabel="Notes on Resources"> + <title>Notes on Resources</title> + + <para> + When an index scan or a sequential scan without sort is used, a temporary + copy of the table is created that contains the table data. Temporary + copies of each index on the table are created as well. Therefore, you need + free space on disk at least equal to the sum of the table size and the + index sizes. + </para> + + <para> + When a sequential scan and sort is used, a temporary sort file is also + created, so that the peak temporary space requirement is as much as double + the table size, plus the index sizes. This method is often faster than + the index scan method, but if the disk space requirement is intolerable, + you can disable this choice by temporarily setting + <xref linkend="guc-enable-sort"/> to <literal>off</literal>. + </para> + + <para> + It is advisable to set <xref linkend="guc-maintenance-work-mem"/> to a + reasonably large value (but not more than the amount of RAM you can + dedicate to the <command>REPACK</command> operation) before repacking. + </para> + </refsect2> + + </refsect1> + + <refsect1> + <title>Examples</title> + + <para> + Repack the table <literal>employees</literal>: +<programlisting> +REPACK employees; +</programlisting> + </para> + + <para> + Repack the table <literal>employees</literal> on the basis of its + index <literal>employees_ind</literal> (Since index is used here, this is + effectively clustering): +<programlisting> +REPACK employees USING INDEX employees_ind; +</programlisting> + </para> + + <para> + Repack all tables in the database on which you have + the <literal>MAINTAIN</literal> privilege: +<programlisting> +REPACK; +</programlisting></para> + </refsect1> + + <refsect1> + <title>Compatibility</title> + + <para> + There is no <command>REPACK</command> statement in the SQL standard. + </para> + + </refsect1> + +</refentry> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index bd5dcaf86a5..735a2a7703a 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -98,6 +98,14 @@ VACUUM [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <re <varlistentry> <term><literal>FULL</literal></term> <listitem> + + <warning> + <para> + The <command>FULL</command> parameter is deprecated in favor of + <xref linkend="sql-repack"/>. + </para> + </warning> + <para> Selects <quote>full</quote> vacuum, which can reclaim more space, but takes much longer and exclusively locks the table. diff --git a/doc/src/sgml/reference.sgml b/doc/src/sgml/reference.sgml index ff85ace83fc..229912d35b7 100644 --- a/doc/src/sgml/reference.sgml +++ b/doc/src/sgml/reference.sgml @@ -195,6 +195,7 @@ &refreshMaterializedView; &reindex; &releaseSavepoint; + &repack; &reset; &revoke; &rollback; diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index ac082fefa77..d91e66241fb 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -741,13 +741,13 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, if (OldIndex != NULL && !use_sort) { const int ci_index[] = { - PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_INDEX_RELID + PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_INDEX_RELID }; int64 ci_val[2]; /* Set phase and OIDOldIndex to columns */ - ci_val[0] = PROGRESS_CLUSTER_PHASE_INDEX_SCAN_HEAP; + ci_val[0] = PROGRESS_REPACK_PHASE_INDEX_SCAN_HEAP; ci_val[1] = RelationGetRelid(OldIndex); pgstat_progress_update_multi_param(2, ci_index, ci_val); @@ -759,15 +759,15 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, else { /* In scan-and-sort mode and also VACUUM FULL, set phase */ - pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP); + pgstat_progress_update_param(PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_PHASE_SEQ_SCAN_HEAP); tableScan = table_beginscan(OldHeap, SnapshotAny, 0, (ScanKey) NULL); heapScan = (HeapScanDesc) tableScan; indexScan = NULL; /* Set total heap blocks */ - pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, + pgstat_progress_update_param(PROGRESS_REPACK_TOTAL_HEAP_BLKS, heapScan->rs_nblocks); } @@ -809,7 +809,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, * is manually updated to the correct value when the table * scan finishes. */ - pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, + pgstat_progress_update_param(PROGRESS_REPACK_HEAP_BLKS_SCANNED, heapScan->rs_nblocks); break; } @@ -825,7 +825,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, */ if (prev_cblock != heapScan->rs_cblock) { - pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, + pgstat_progress_update_param(PROGRESS_REPACK_HEAP_BLKS_SCANNED, (heapScan->rs_cblock + heapScan->rs_nblocks - heapScan->rs_startblock @@ -912,14 +912,14 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, * In scan-and-sort mode, report increase in number of tuples * scanned */ - pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED, + pgstat_progress_update_param(PROGRESS_REPACK_HEAP_TUPLES_SCANNED, *num_tuples); } else { const int ct_index[] = { - PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED, - PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN + PROGRESS_REPACK_HEAP_TUPLES_SCANNED, + PROGRESS_REPACK_HEAP_TUPLES_WRITTEN }; int64 ct_val[2]; @@ -952,14 +952,14 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, double n_tuples = 0; /* Report that we are now sorting tuples */ - pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_PHASE_SORT_TUPLES); + pgstat_progress_update_param(PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_PHASE_SORT_TUPLES); tuplesort_performsort(tuplesort); /* Report that we are now writing new heap */ - pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_PHASE_WRITE_NEW_HEAP); + pgstat_progress_update_param(PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_PHASE_WRITE_NEW_HEAP); for (;;) { @@ -977,7 +977,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, values, isnull, rwstate); /* Report n_tuples */ - pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN, + pgstat_progress_update_param(PROGRESS_REPACK_HEAP_TUPLES_WRITTEN, n_tuples); } diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 739a92bdcc1..466cf0fdef6 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -4079,7 +4079,7 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, Assert(!ReindexIsProcessingIndex(indexOid)); /* Set index rebuild count */ - pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT, + pgstat_progress_update_param(PROGRESS_REPACK_INDEX_REBUILD_COUNT, i); i++; } diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 273008db37f..1d2ea145fe7 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1263,6 +1263,33 @@ CREATE VIEW pg_stat_progress_cluster AS FROM pg_stat_get_progress_info('CLUSTER') AS S LEFT JOIN pg_database D ON S.datid = D.oid; +CREATE VIEW pg_stat_progress_repack AS + SELECT + S.pid AS pid, + S.datid AS datid, + D.datname AS datname, + S.relid AS relid, + CASE S.param1 WHEN 1 THEN 'REPACK' + END AS command, + CASE S.param2 WHEN 0 THEN 'initializing' + WHEN 1 THEN 'seq scanning heap' + WHEN 2 THEN 'index scanning heap' + WHEN 3 THEN 'sorting tuples' + WHEN 4 THEN 'writing new heap' + WHEN 5 THEN 'swapping relation files' + WHEN 6 THEN 'rebuilding index' + WHEN 7 THEN 'performing final cleanup' + END AS phase, + CAST(S.param3 AS oid) AS repack_index_relid, + S.param4 AS heap_tuples_scanned, + S.param5 AS heap_tuples_written, + S.param6 AS heap_blks_total, + S.param7 AS heap_blks_scanned, + S.param8 AS index_rebuild_count + FROM pg_stat_get_progress_info('REPACK') AS S + LEFT JOIN pg_database D ON S.datid = D.oid; + + CREATE VIEW pg_stat_progress_create_index AS SELECT S.pid AS pid, S.datid AS datid, D.datname AS datname, diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 54a08e4102e..dab6499127e 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -67,17 +67,21 @@ typedef struct Oid indexOid; } RelToCluster; - -static void cluster_multiple_rels(List *rtcs, ClusterParams *params); +static void cluster_multiple_rels(List *rtcs, ClusterParams *params, + ClusterCommand cmd); static void rebuild_relation(Relation OldHeap, Relation index, bool verbose); static void copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); static List *get_tables_to_cluster(MemoryContext cluster_context); +static List *get_tables_to_repack(MemoryContext repack_context); static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context, - Oid indexOid); + Oid relid, bool rel_is_index); static bool cluster_is_permitted_for_relation(Oid relid, Oid userid); - +static Relation process_single_relation(RangeVar *relation, char *indexname, + ClusterParams *params, + ClusterCommand cmd, + Oid *indexOid_p); /*--------------------------------------------------------------------------- * This cluster code allows for clustering multiple tables at once. Because @@ -134,71 +138,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) if (stmt->relation != NULL) { /* This is the single-relation case. */ - Oid tableOid; - - /* - * Find, lock, and check permissions on the table. We obtain - * AccessExclusiveLock right away to avoid lock-upgrade hazard in the - * single-transaction case. - */ - tableOid = RangeVarGetRelidExtended(stmt->relation, - AccessExclusiveLock, - 0, - RangeVarCallbackMaintainsTable, - NULL); - rel = table_open(tableOid, NoLock); - - /* - * Reject clustering a remote temp table ... their local buffer - * manager is not going to cope. - */ - if (RELATION_IS_OTHER_TEMP(rel)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot cluster temporary tables of other sessions"))); - - if (stmt->indexname == NULL) - { - ListCell *index; - - /* We need to find the index that has indisclustered set. */ - foreach(index, RelationGetIndexList(rel)) - { - indexOid = lfirst_oid(index); - if (get_index_isclustered(indexOid)) - break; - indexOid = InvalidOid; - } - - if (!OidIsValid(indexOid)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("there is no previously clustered index for table \"%s\"", - stmt->relation->relname))); - } - else - { - /* - * The index is expected to be in the same namespace as the - * relation. - */ - indexOid = get_relname_relid(stmt->indexname, - rel->rd_rel->relnamespace); - if (!OidIsValid(indexOid)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("index \"%s\" for table \"%s\" does not exist", - stmt->indexname, stmt->relation->relname))); - } - - /* For non-partitioned tables, do what we came here to do. */ - if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) - { - cluster_rel(rel, indexOid, ¶ms); - /* cluster_rel closes the relation, but keeps lock */ - + rel = process_single_relation(stmt->relation, stmt->indexname, + ¶ms, CLUSTER_COMMAND_CLUSTER, + &indexOid); + if (rel == NULL) return; - } } /* @@ -231,7 +175,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); check_index_is_clusterable(rel, indexOid, AccessShareLock); - rtcs = get_tables_to_cluster_partitioned(cluster_context, indexOid); + rtcs = get_tables_to_cluster_partitioned(cluster_context, indexOid, + true); /* close relation, releasing lock on parent table */ table_close(rel, AccessExclusiveLock); @@ -243,7 +188,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) } /* Do the job. */ - cluster_multiple_rels(rtcs, ¶ms); + cluster_multiple_rels(rtcs, ¶ms, CLUSTER_COMMAND_CLUSTER); /* Start a new transaction for the cleanup work. */ StartTransactionCommand(); @@ -260,7 +205,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) * return. */ static void -cluster_multiple_rels(List *rtcs, ClusterParams *params) +cluster_multiple_rels(List *rtcs, ClusterParams *params, ClusterCommand cmd) { ListCell *lc; @@ -283,7 +228,7 @@ cluster_multiple_rels(List *rtcs, ClusterParams *params) rel = table_open(rtc->tableOid, AccessExclusiveLock); /* Process this table */ - cluster_rel(rel, rtc->indexOid, params); + cluster_rel(rel, rtc->indexOid, params, cmd); /* cluster_rel closes the relation, but keeps lock */ PopActiveSnapshot(); @@ -306,9 +251,13 @@ cluster_multiple_rels(List *rtcs, ClusterParams *params) * If indexOid is InvalidOid, the table will be rewritten in physical order * instead of index order. This is the new implementation of VACUUM FULL, * and error messages should refer to the operation as VACUUM not CLUSTER. + * + * 'cmd' indicates which commands is being executed. REPACK should be the only + * caller of this function in the future. */ void -cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params) +cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params, + ClusterCommand cmd) { Oid tableOid = RelationGetRelid(OldHeap); Oid save_userid; @@ -323,13 +272,26 @@ cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params) /* Check for user-requested abort. */ CHECK_FOR_INTERRUPTS(); - pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tableOid); - if (OidIsValid(indexOid)) - pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND, + if (cmd == CLUSTER_COMMAND_REPACK) + pgstat_progress_start_command(PROGRESS_COMMAND_REPACK, tableOid); + else + pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tableOid); + + if (cmd == CLUSTER_COMMAND_REPACK) + pgstat_progress_update_param(PROGRESS_REPACK_COMMAND, + PROGRESS_REPACK_COMMAND_REPACK); + else if (OidIsValid(indexOid)) + { + Assert(cmd == CLUSTER_COMMAND_CLUSTER); + pgstat_progress_update_param(PROGRESS_REPACK_COMMAND, PROGRESS_CLUSTER_COMMAND_CLUSTER); + } else - pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND, + { + Assert(cmd == CLUSTER_COMMAND_VACUUM); + pgstat_progress_update_param(PROGRESS_REPACK_COMMAND, PROGRESS_CLUSTER_COMMAND_VACUUM_FULL); + } /* * Switch to the table owner's userid, so that any index functions are run @@ -403,8 +365,12 @@ cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params) * would work in most respects, but the index would only get marked as * indisclustered in the current database, leading to unexpected behavior * if CLUSTER were later invoked in another database. + * + * REPACK does not set indisclustered. XXX Not sure I understand the + * comment above: how can an attribute be set "only in the current + * database"? */ - if (OidIsValid(indexOid) && OldHeap->rd_rel->relisshared) + if (cmd == CLUSTER_COMMAND_CLUSTER && OldHeap->rd_rel->relisshared) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot cluster a shared catalog"))); @@ -415,21 +381,19 @@ cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params) */ if (RELATION_IS_OTHER_TEMP(OldHeap)) { - if (OidIsValid(indexOid)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot cluster temporary tables of other sessions"))); - else - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot vacuum temporary tables of other sessions"))); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot cluster temporary tables of other sessions"))); } /* * Also check for active uses of the relation in the current transaction, * including open scans and pending AFTER trigger events. */ - CheckTableNotInUse(OldHeap, OidIsValid(indexOid) ? "CLUSTER" : "VACUUM"); + CheckTableNotInUse(OldHeap, + (cmd == CLUSTER_COMMAND_CLUSTER ? + "CLUSTER" : (cmd == CLUSTER_COMMAND_REPACK ? + "REPACK" : "VACUUM"))); /* Check heap and index are valid to cluster on */ if (OidIsValid(indexOid)) @@ -1458,8 +1422,8 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, int i; /* Report that we are now swapping relation files */ - pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES); + pgstat_progress_update_param(PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_PHASE_SWAP_REL_FILES); /* Zero out possible results from swapped_relation_files */ memset(mapped_tables, 0, sizeof(mapped_tables)); @@ -1509,14 +1473,14 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, reindex_flags |= REINDEX_REL_FORCE_INDEXES_PERMANENT; /* Report that we are now reindexing relations */ - pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_PHASE_REBUILD_INDEX); + pgstat_progress_update_param(PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_PHASE_REBUILD_INDEX); reindex_relation(NULL, OIDOldHeap, reindex_flags, &reindex_params); /* Report that we are now doing clean up */ - pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, - PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP); + pgstat_progress_update_param(PROGRESS_REPACK_PHASE, + PROGRESS_REPACK_PHASE_FINAL_CLEANUP); /* * If the relation being rebuilt is pg_class, swap_relation_files() @@ -1687,14 +1651,66 @@ get_tables_to_cluster(MemoryContext cluster_context) } /* - * Given an index on a partitioned table, return a list of RelToCluster for + * Like get_tables_to_cluster(), but do not care about indexes. + */ +static List * +get_tables_to_repack(MemoryContext repack_context) +{ + Relation relrelation; + TableScanDesc scan; + HeapTuple tuple; + MemoryContext old_context; + List *rtcs = NIL; + + /* + * Get all relations that the current user has the appropriate privileges + * for. + */ + relrelation = table_open(RelationRelationId, AccessShareLock); + scan = table_beginscan_catalog(relrelation, 0, NULL); + while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) + { + RelToCluster *rtc; + Form_pg_class relrelation = (Form_pg_class) GETSTRUCT(tuple); + Oid relid = relrelation->oid; + + /* Only interested in relations. */ + if (get_rel_relkind(relid) != RELKIND_RELATION) + continue; + + if (!cluster_is_permitted_for_relation(relid, GetUserId())) + continue; + + /* Use a permanent memory context for the result list */ + old_context = MemoryContextSwitchTo(repack_context); + + rtc = (RelToCluster *) palloc(sizeof(RelToCluster)); + rtc->tableOid = relid; + rtc->indexOid = InvalidOid; + rtcs = lappend(rtcs, rtc); + + MemoryContextSwitchTo(old_context); + } + table_endscan(scan); + + relation_close(relrelation, AccessShareLock); + + return rtcs; +} + +/* + * Given a partitioned table or its index, return a list of RelToCluster for * all the children leaves tables/indexes. * * Like expand_vacuum_rel, but here caller must hold AccessExclusiveLock * on the table containing the index. + * + * 'rel_is_index' tells whether 'relid' is that of an index (true) or of the + * owning relation. */ static List * -get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) +get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid relid, + bool rel_is_index) { List *inhoids; ListCell *lc; @@ -1702,17 +1718,33 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) MemoryContext old_context; /* Do not lock the children until they're processed */ - inhoids = find_all_inheritors(indexOid, NoLock, NULL); + inhoids = find_all_inheritors(relid, NoLock, NULL); foreach(lc, inhoids) { - Oid indexrelid = lfirst_oid(lc); - Oid relid = IndexGetRelation(indexrelid, false); + Oid inhoid = lfirst_oid(lc); + Oid inhrelid, + inhindid; RelToCluster *rtc; - /* consider only leaf indexes */ - if (get_rel_relkind(indexrelid) != RELKIND_INDEX) - continue; + if (rel_is_index) + { + /* consider only leaf indexes */ + if (get_rel_relkind(inhoid) != RELKIND_INDEX) + continue; + + inhrelid = IndexGetRelation(inhoid, false); + inhindid = inhoid; + } + else + { + /* consider only leaf relations */ + if (get_rel_relkind(inhoid) != RELKIND_RELATION) + continue; + + inhrelid = inhoid; + inhindid = InvalidOid; + } /* * It's possible that the user does not have privileges to CLUSTER the @@ -1720,15 +1752,15 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) * table. We skip any partitions which the user is not permitted to * CLUSTER. */ - if (!cluster_is_permitted_for_relation(relid, GetUserId())) + if (!cluster_is_permitted_for_relation(inhrelid, GetUserId())) continue; /* Use a permanent memory context for the result list */ old_context = MemoryContextSwitchTo(cluster_context); rtc = (RelToCluster *) palloc(sizeof(RelToCluster)); - rtc->tableOid = relid; - rtc->indexOid = indexrelid; + rtc->tableOid = inhrelid; + rtc->indexOid = inhindid; rtcs = lappend(rtcs, rtc); MemoryContextSwitchTo(old_context); @@ -1752,3 +1784,179 @@ cluster_is_permitted_for_relation(Oid relid, Oid userid) get_rel_name(relid)))); return false; } + +/* + * REPACK is intended to be a replacement of both CLUSTER and VACUUM FULL. + */ +void +repack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel) +{ + ListCell *lc; + ClusterParams params = {0}; + bool verbose = false; + Relation rel = NULL; + Oid indexOid = InvalidOid; + MemoryContext repack_context; + List *rtcs; + + /* Parse option list */ + foreach(lc, stmt->params) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + verbose = defGetBoolean(opt); + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("unrecognized REPACK option \"%s\"", + opt->defname), + parser_errposition(pstate, opt->location))); + } + + params.options = (verbose ? CLUOPT_VERBOSE : 0); + + if (stmt->relation != NULL) + { + /* This is the single-relation case. */ + rel = process_single_relation(stmt->relation, stmt->indexname, + ¶ms, CLUSTER_COMMAND_REPACK, + &indexOid); + if (rel == NULL) + return; + } + + /* + * By here, we know we are in a multi-table situation. In order to avoid + * holding locks for too long, we want to process each table in its own + * transaction. This forces us to disallow running inside a user + * transaction block. + */ + PreventInTransactionBlock(isTopLevel, "REPACK"); + + /* Also, we need a memory context to hold our list of relations */ + repack_context = AllocSetContextCreate(PortalContext, + "Repack", + ALLOCSET_DEFAULT_SIZES); + + params.options |= CLUOPT_RECHECK; + if (rel != NULL) + { + Oid relid; + bool rel_is_index; + + Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); + + if (OidIsValid(indexOid)) + { + relid = indexOid; + rel_is_index = true; + } + else + { + relid = RelationGetRelid(rel); + rel_is_index = false; + } + rtcs = get_tables_to_cluster_partitioned(repack_context, relid, + rel_is_index); + + /* close relation, releasing lock on parent table */ + table_close(rel, AccessExclusiveLock); + } + else + rtcs = get_tables_to_repack(repack_context); + + /* Do the job. */ + cluster_multiple_rels(rtcs, ¶ms, CLUSTER_COMMAND_REPACK); + + /* Start a new transaction for the cleanup work. */ + StartTransactionCommand(); + + /* Clean up working storage */ + MemoryContextDelete(repack_context); + +} + +/* + * REPACK a single relation if it's a non-partitioned table or a leaf + * partition and return NULL. Return the relation's relcache entry if the + * caller needs to process it (because the relation is partitioned). + */ +static Relation +process_single_relation(RangeVar *relation, char *indexname, + ClusterParams *params, ClusterCommand cmd, + Oid *indexOid_p) +{ + Relation rel; + Oid indexOid = InvalidOid; + + /* This is the single-relation case. */ + Oid tableOid; + + /* + * Find, lock, and check permissions on the table. We obtain + * AccessExclusiveLock right away to avoid lock-upgrade hazard in the + * single-transaction case. + */ + tableOid = RangeVarGetRelidExtended(relation, + AccessExclusiveLock, + 0, + RangeVarCallbackMaintainsTable, + NULL); + rel = table_open(tableOid, NoLock); + + /* + * Reject clustering a remote temp table ... their local buffer manager is + * not going to cope. + */ + if (RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot cluster temporary tables of other sessions"))); + + if (indexname == NULL && cmd == CLUSTER_COMMAND_CLUSTER) + { + ListCell *index; + + /* We need to find the index that has indisclustered set. */ + foreach(index, RelationGetIndexList(rel)) + { + indexOid = lfirst_oid(index); + if (get_index_isclustered(indexOid)) + break; + indexOid = InvalidOid; + } + + if (!OidIsValid(indexOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("there is no previously clustered index for table \"%s\"", + relation->relname))); + } + else if (indexname != NULL) + { + /* + * The index is expected to be in the same namespace as the relation. + */ + indexOid = get_relname_relid(indexname, + rel->rd_rel->relnamespace); + if (!OidIsValid(indexOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("index \"%s\" for table \"%s\" does not exist", + indexname, relation->relname))); + } + + *indexOid_p = indexOid; + + /* For non-partitioned tables, do what we came here to do. */ + if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + { + cluster_rel(rel, indexOid, params, cmd); + /* cluster_rel closes the relation, but keeps lock */ + + return NULL; + } + + return rel; +} diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index db5da3ce826..a4ad23448f8 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2263,7 +2263,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, cluster_params.options |= CLUOPT_VERBOSE; /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ - cluster_rel(rel, InvalidOid, &cluster_params); + cluster_rel(rel, InvalidOid, &cluster_params, + CLUSTER_COMMAND_VACUUM); /* cluster_rel closes the relation, but keeps lock */ rel = NULL; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index f1156e2fca3..ccf630edbb9 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -298,7 +298,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); GrantStmt GrantRoleStmt ImportForeignSchemaStmt IndexStmt InsertStmt ListenStmt LoadStmt LockStmt MergeStmt NotifyStmt ExplainableStmt PreparableStmt CreateFunctionStmt AlterFunctionStmt ReindexStmt RemoveAggrStmt - RemoveFuncStmt RemoveOperStmt RenameStmt ReturnStmt RevokeStmt RevokeRoleStmt + RemoveFuncStmt RemoveOperStmt RenameStmt RepackStmt ReturnStmt RevokeStmt RevokeRoleStmt RuleActionStmt RuleActionStmtOrEmpty RuleStmt SecLabelStmt SelectStmt TransactionStmt TransactionStmtLegacy TruncateStmt UnlistenStmt UpdateStmt VacuumStmt @@ -381,7 +381,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <str> copy_file_name access_method_clause attr_name table_access_method_clause name cursor_name file_name - cluster_index_specification + cluster_index_specification repack_index_specification %type <list> func_name handler_name qual_Op qual_all_Op subquery_Op opt_inline_handler opt_validator validator_clause @@ -764,7 +764,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); QUOTE QUOTES RANGE READ REAL REASSIGN RECURSIVE REF_P REFERENCES REFERENCING - REFRESH REINDEX RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA + REFRESH REINDEX RELATIVE_P RELEASE RENAME REPACK REPEATABLE REPLACE REPLICA RESET RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP ROUTINE ROUTINES ROW ROWS RULE @@ -1100,6 +1100,7 @@ stmt: | RemoveFuncStmt | RemoveOperStmt | RenameStmt + | RepackStmt | RevokeStmt | RevokeRoleStmt | RuleStmt @@ -11893,6 +11894,60 @@ cluster_index_specification: | /*EMPTY*/ { $$ = NULL; } ; +/***************************************************************************** + * + * QUERY: + * REPACK [ (options) ] [ <qualified_name> [ USING INDEX <index_name> ] ] + * + *****************************************************************************/ + +RepackStmt: + REPACK qualified_name repack_index_specification + { + RepackStmt *n = makeNode(RepackStmt); + + n->relation = $2; + n->indexname = $3; + n->params = NIL; + $$ = (Node *) n; + } + + | REPACK '(' utility_option_list ')' qualified_name repack_index_specification + { + RepackStmt *n = makeNode(RepackStmt); + + n->relation = $5; + n->indexname = $6; + n->params = $3; + $$ = (Node *) n; + } + + | REPACK + { + RepackStmt *n = makeNode(RepackStmt); + + n->relation = NULL; + n->indexname = NULL; + n->params = NIL; + $$ = (Node *) n; + } + + | REPACK '(' utility_option_list ')' + { + RepackStmt *n = makeNode(RepackStmt); + + n->relation = NULL; + n->indexname = NULL; + n->params = $3; + $$ = (Node *) n; + } + ; + +repack_index_specification: + USING INDEX name { $$ = $3; } + | /*EMPTY*/ { $$ = NULL; } + ; + /***************************************************************************** * @@ -17934,6 +17989,7 @@ unreserved_keyword: | RELATIVE_P | RELEASE | RENAME + | REPACK | REPEATABLE | REPLACE | REPLICA @@ -18566,6 +18622,7 @@ bare_label_keyword: | RELATIVE_P | RELEASE | RENAME + | REPACK | REPEATABLE | REPLACE | REPLICA diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 25fe3d58016..bf3ba3c2ae7 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -280,6 +280,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree) case T_ClusterStmt: case T_ReindexStmt: case T_VacuumStmt: + case T_RepackStmt: { /* * These commands write WAL, so they're not strictly @@ -862,6 +863,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, ExecVacuum(pstate, (VacuumStmt *) parsetree, isTopLevel); break; + case T_RepackStmt: + repack(pstate, (RepackStmt *) parsetree, isTopLevel); + break; + case T_ExplainStmt: ExplainQuery(pstate, (ExplainStmt *) parsetree, params, dest); break; @@ -2869,6 +2874,10 @@ CreateCommandTag(Node *parsetree) tag = CMDTAG_ANALYZE; break; + case T_RepackStmt: + tag = CMDTAG_REPACK; + break; + case T_ExplainStmt: tag = CMDTAG_EXPLAIN; break; diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 97af7c6554f..ddec4914ea5 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -268,6 +268,8 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) cmdtype = PROGRESS_COMMAND_ANALYZE; else if (pg_strcasecmp(cmd, "CLUSTER") == 0) cmdtype = PROGRESS_COMMAND_CLUSTER; + else if (pg_strcasecmp(cmd, "REPACK") == 0) + cmdtype = PROGRESS_COMMAND_REPACK; else if (pg_strcasecmp(cmd, "CREATE INDEX") == 0) cmdtype = PROGRESS_COMMAND_CREATE_INDEX; else if (pg_strcasecmp(cmd, "BASEBACKUP") == 0) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index c916b9299a8..8512e099b03 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -1223,7 +1223,7 @@ static const char *const sql_commands[] = { "DELETE FROM", "DISCARD", "DO", "DROP", "END", "EXECUTE", "EXPLAIN", "FETCH", "GRANT", "IMPORT FOREIGN SCHEMA", "INSERT INTO", "LISTEN", "LOAD", "LOCK", "MERGE INTO", "MOVE", "NOTIFY", "PREPARE", - "REASSIGN", "REFRESH MATERIALIZED VIEW", "REINDEX", "RELEASE", + "REASSIGN", "REFRESH MATERIALIZED VIEW", "REINDEX", "RELEASE", "REPACK", "RESET", "REVOKE", "ROLLBACK", "SAVEPOINT", "SECURITY LABEL", "SELECT", "SET", "SHOW", "START", "TABLE", "TRUNCATE", "UNLISTEN", "UPDATE", "VACUUM", "VALUES", "WITH", @@ -4913,6 +4913,35 @@ match_previous_words(int pattern_id, COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); } +/* REPACK */ + else if (Matches("REPACK")) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables); + else if (Matches("REPACK", "(*)")) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables); + /* If we have REPACK <sth>, then add "USING INDEX" */ + else if (Matches("REPACK", MatchAnyExcept("("))) + COMPLETE_WITH("USING INDEX"); + /* If we have REPACK (*) <sth>, then add "USING INDEX" */ + else if (Matches("REPACK", "(*)", MatchAny)) + COMPLETE_WITH("USING INDEX"); + /* If we have REPACK <sth> USING, then add the index as well */ + else if (Matches("REPACK", MatchAny, "USING", "INDEX")) + { + set_completion_reference(prev3_wd); + COMPLETE_WITH_SCHEMA_QUERY(Query_for_index_of_table); + } + else if (HeadMatches("REPACK", "(*") && + !HeadMatches("REPACK", "(*)")) + { + /* + * This fires if we're in an unfinished parenthesized option list. + * get_previous_words treats a completed parenthesized option list as + * one word, so the above test is correct. + */ + if (ends_with(prev_wd, '(') || ends_with(prev_wd, ',')) + COMPLETE_WITH("VERBOSE"); + } + /* SECURITY LABEL */ else if (Matches("SECURITY")) COMPLETE_WITH("LABEL"); diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index 60088a64cbb..3be57c97b3f 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -31,8 +31,24 @@ typedef struct ClusterParams bits32 options; /* bitmask of CLUOPT_* */ } ClusterParams; +/* + * cluster.c currently implements three nearly identical commands: CLUSTER, + * VACUUM FULL and REPACK. Where needed, use this enumeration to distinguish + * which of these commands is being executed. + * + * Remove this stuff when removing the (now deprecated) CLUSTER and VACUUM + * FULL commands. + */ +typedef enum ClusterCommand +{ + CLUSTER_COMMAND_CLUSTER, + CLUSTER_COMMAND_REPACK, + CLUSTER_COMMAND_VACUUM +} ClusterCommand; + extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel); -extern void cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params); +extern void cluster_rel(Relation OldHeap, Oid indexOid, ClusterParams *params, + ClusterCommand cmd); extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid, LOCKMODE lockmode); extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal); @@ -48,4 +64,5 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, MultiXactId cutoffMulti, char newrelpersistence); +extern void repack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel); #endif /* CLUSTER_H */ diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h index 7c736e7b03b..7644267e14f 100644 --- a/src/include/commands/progress.h +++ b/src/include/commands/progress.h @@ -56,24 +56,48 @@ #define PROGRESS_ANALYZE_PHASE_COMPUTE_EXT_STATS 4 #define PROGRESS_ANALYZE_PHASE_FINALIZE_ANALYZE 5 -/* Progress parameters for cluster */ -#define PROGRESS_CLUSTER_COMMAND 0 -#define PROGRESS_CLUSTER_PHASE 1 -#define PROGRESS_CLUSTER_INDEX_RELID 2 -#define PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED 3 -#define PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN 4 -#define PROGRESS_CLUSTER_TOTAL_HEAP_BLKS 5 -#define PROGRESS_CLUSTER_HEAP_BLKS_SCANNED 6 -#define PROGRESS_CLUSTER_INDEX_REBUILD_COUNT 7 - -/* Phases of cluster (as advertised via PROGRESS_CLUSTER_PHASE) */ -#define PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP 1 -#define PROGRESS_CLUSTER_PHASE_INDEX_SCAN_HEAP 2 -#define PROGRESS_CLUSTER_PHASE_SORT_TUPLES 3 -#define PROGRESS_CLUSTER_PHASE_WRITE_NEW_HEAP 4 -#define PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES 5 -#define PROGRESS_CLUSTER_PHASE_REBUILD_INDEX 6 -#define PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP 7 +/* + * Progress parameters for REPACK. + * + * Note: Since REPACK shares some code with CLUSTER, (some of) these values + * are also used by CLUSTER. (CLUSTER is now deprecated, so it makes no sense + * to introduce separate set of constants.) + */ +#define PROGRESS_REPACK_COMMAND 0 +#define PROGRESS_REPACK_PHASE 1 +#define PROGRESS_REPACK_INDEX_RELID 2 +#define PROGRESS_REPACK_HEAP_TUPLES_SCANNED 3 +#define PROGRESS_REPACK_HEAP_TUPLES_WRITTEN 4 +#define PROGRESS_REPACK_TOTAL_HEAP_BLKS 5 +#define PROGRESS_REPACK_HEAP_BLKS_SCANNED 6 +#define PROGRESS_REPACK_INDEX_REBUILD_COUNT 7 + +/* + * Phases of repack (as advertised via PROGRESS_REPACK_PHASE). + * + * Note: Since REPACK shares some code with CLUSTER, (some of) these values + * are also used by CLUSTER. (CLUSTER is now deprecated, so it makes no sense + * to introduce separate set of constants.) + */ +#define PROGRESS_REPACK_PHASE_SEQ_SCAN_HEAP 1 +#define PROGRESS_REPACK_PHASE_INDEX_SCAN_HEAP 2 +#define PROGRESS_REPACK_PHASE_SORT_TUPLES 3 +#define PROGRESS_REPACK_PHASE_WRITE_NEW_HEAP 4 +#define PROGRESS_REPACK_PHASE_SWAP_REL_FILES 5 +#define PROGRESS_REPACK_PHASE_REBUILD_INDEX 6 +#define PROGRESS_REPACK_PHASE_FINAL_CLEANUP 7 + +/* Commands of PROGRESS_REPACK */ +#define PROGRESS_REPACK_COMMAND_REPACK 1 + +/* + * Progress parameters for cluster. + * + * Although we need to report REPACK and CLUSTER in separate views, the + * parameters and phases of CLUSTER are a subset of those of REPACK. Therefore + * we just use the appropriate values defined for REPACK above instead of + * defining a separate set of constants here. + */ /* Commands of PROGRESS_CLUSTER */ #define PROGRESS_CLUSTER_COMMAND_CLUSTER 1 diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 4610fc61293..648484205cb 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3923,6 +3923,19 @@ typedef struct ClusterStmt List *params; /* list of DefElem nodes */ } ClusterStmt; +/* ---------------------- + * Repack Statement + * ---------------------- + */ +typedef struct RepackStmt +{ + NodeTag type; + RangeVar *relation; /* relation being repacked */ + char *indexname; /* order tuples by this index */ + List *params; /* list of DefElem nodes */ +} RepackStmt; + + /* ---------------------- * Vacuum and Analyze Statements * diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index a4af3f717a1..22559369e2c 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -374,6 +374,7 @@ PG_KEYWORD("reindex", REINDEX, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("relative", RELATIVE_P, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("release", RELEASE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("rename", RENAME, UNRESERVED_KEYWORD, BARE_LABEL) +PG_KEYWORD("repack", REPACK, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("repeatable", REPEATABLE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("replace", REPLACE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("replica", REPLICA, UNRESERVED_KEYWORD, BARE_LABEL) diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h index d250a714d59..cceb312f2b3 100644 --- a/src/include/tcop/cmdtaglist.h +++ b/src/include/tcop/cmdtaglist.h @@ -196,6 +196,7 @@ PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false) PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false) PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false) PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false) +PG_CMDTAG(CMDTAG_REPACK, "REPACK", false, false, false) PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false) PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false) PG_CMDTAG(CMDTAG_REVOKE_ROLE, "REVOKE ROLE", false, false, false) diff --git a/src/include/utils/backend_progress.h b/src/include/utils/backend_progress.h index dda813ab407..e69e366dcdc 100644 --- a/src/include/utils/backend_progress.h +++ b/src/include/utils/backend_progress.h @@ -28,6 +28,7 @@ typedef enum ProgressCommandType PROGRESS_COMMAND_CREATE_INDEX, PROGRESS_COMMAND_BASEBACKUP, PROGRESS_COMMAND_COPY, + PROGRESS_COMMAND_REPACK, } ProgressCommandType; #define PGSTAT_NUM_PROGRESS_PARAM 20 diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 4d40a6809ab..e9fd7512710 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -254,6 +254,63 @@ ORDER BY 1; clstr_tst_pkey (3 rows) +-- REPACK handles individual tables identically to CLUSTER, but it's worth +-- checking if it handles table hierarchies identically as well. +REPACK clstr_tst USING INDEX clstr_tst_c; +-- Verify that inheritance link still works +INSERT INTO clstr_tst_inh VALUES (0, 100, 'in child table 2'); +SELECT a,b,c,substring(d for 30), length(d) from clstr_tst; + a | b | c | substring | length +----+-----+------------------+--------------------------------+-------- + 10 | 14 | catorce | | + 18 | 5 | cinco | | + 9 | 4 | cuatro | | + 26 | 19 | diecinueve | | + 12 | 18 | dieciocho | | + 30 | 16 | dieciseis | | + 24 | 17 | diecisiete | | + 2 | 10 | diez | | + 23 | 12 | doce | | + 11 | 2 | dos | | + 25 | 9 | nueve | | + 31 | 8 | ocho | | + 1 | 11 | once | | + 28 | 15 | quince | | + 32 | 6 | seis | xyzzyxyzzyxyzzyxyzzyxyzzyxyzzy | 500000 + 29 | 7 | siete | | + 15 | 13 | trece | | + 22 | 30 | treinta | | + 17 | 32 | treinta y dos | | + 3 | 31 | treinta y uno | | + 5 | 3 | tres | | + 20 | 1 | uno | | + 6 | 20 | veinte | | + 14 | 25 | veinticinco | | + 21 | 24 | veinticuatro | | + 4 | 22 | veintidos | | + 19 | 29 | veintinueve | | + 16 | 28 | veintiocho | | + 27 | 26 | veintiseis | | + 13 | 27 | veintisiete | | + 7 | 23 | veintitres | | + 8 | 21 | veintiuno | | + 0 | 100 | in child table | | + 0 | 100 | in child table 2 | | +(34 rows) + +-- Verify that foreign key link still works +INSERT INTO clstr_tst (b, c) VALUES (1111, 'this should fail'); +ERROR: insert or update on table "clstr_tst" violates foreign key constraint "clstr_tst_con" +DETAIL: Key (b)=(1111) is not present in table "clstr_tst_s". +SELECT conname FROM pg_constraint WHERE conrelid = 'clstr_tst'::regclass +ORDER BY 1; + conname +---------------------- + clstr_tst_a_not_null + clstr_tst_con + clstr_tst_pkey +(3 rows) + SELECT relname, relkind, EXISTS(SELECT 1 FROM pg_class WHERE oid = c.reltoastrelid) AS hastoast FROM pg_class c WHERE relname LIKE 'clstr_tst%' ORDER BY relname; @@ -381,6 +438,35 @@ SELECT * FROM clstr_1; 2 (2 rows) +-- REPACK w/o argument performs no ordering, so we can only check which tables +-- have the relfilenode changed. +RESET SESSION AUTHORIZATION; +CREATE TEMP TABLE relnodes_old AS +(SELECT relname, relfilenode +FROM pg_class +WHERE relname IN ('clstr_1', 'clstr_2', 'clstr_3')); +SET SESSION AUTHORIZATION regress_clstr_user; +SET client_min_messages = ERROR; -- order of "skipping" warnings may vary +REPACK; +RESET client_min_messages; +RESET SESSION AUTHORIZATION; +CREATE TEMP TABLE relnodes_new AS +(SELECT relname, relfilenode +FROM pg_class +WHERE relname IN ('clstr_1', 'clstr_2', 'clstr_3')); +-- Do the actual comparison. Unlike CLUSTER, clstr_3 should have been +-- processed because there is nothing like clustering index here. +SELECT o.relname FROM relnodes_old o +JOIN relnodes_new n ON o.relname = n.relname +WHERE o.relfilenode <> n.relfilenode +ORDER BY o.relname; + relname +--------- + clstr_1 + clstr_3 +(2 rows) + +SET SESSION AUTHORIZATION regress_clstr_user; -- Test MVCC-safety of cluster. There isn't much we can do to verify the -- results with a single backend... CREATE TABLE clustertest (key int PRIMARY KEY); @@ -495,6 +581,43 @@ ALTER TABLE clstrpart SET WITHOUT CLUSTER; ERROR: cannot mark index clustered in partitioned table ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; ERROR: cannot mark index clustered in partitioned table +-- Check that REPACK sets new relfilenodes: it should process exactly the same +-- tables as CLUSTER did. +DROP TABLE old_cluster_info; +DROP TABLE new_cluster_info; +CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +REPACK clstrpart USING INDEX clstrpart_idx; +CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C"; + relname | level | relkind | ?column? +-------------+-------+---------+---------- + clstrpart | 0 | p | t + clstrpart1 | 1 | p | t + clstrpart11 | 2 | r | f + clstrpart12 | 2 | p | t + clstrpart2 | 1 | r | f + clstrpart3 | 1 | p | t + clstrpart33 | 2 | r | f +(7 rows) + +-- And finally the same for REPACK w/o index. +DROP TABLE old_cluster_info; +DROP TABLE new_cluster_info; +CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +REPACK clstrpart; +CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C"; + relname | level | relkind | ?column? +-------------+-------+---------+---------- + clstrpart | 0 | p | t + clstrpart1 | 1 | p | t + clstrpart11 | 2 | r | f + clstrpart12 | 2 | p | t + clstrpart2 | 1 | r | f + clstrpart3 | 1 | p | t + clstrpart33 | 2 | r | f +(7 rows) + DROP TABLE clstrpart; -- Ownership of partitions is checked CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 673c63b8d1b..e7513e64fd2 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2058,6 +2058,33 @@ pg_stat_progress_create_index| SELECT s.pid, s.param15 AS partitions_done FROM (pg_stat_get_progress_info('CREATE INDEX'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20) LEFT JOIN pg_database d ON ((s.datid = d.oid))); +pg_stat_progress_repack| SELECT s.pid, + s.datid, + d.datname, + s.relid, + CASE s.param1 + WHEN 1 THEN 'REPACK'::text + ELSE NULL::text + END AS command, + CASE s.param2 + WHEN 0 THEN 'initializing'::text + WHEN 1 THEN 'seq scanning heap'::text + WHEN 2 THEN 'index scanning heap'::text + WHEN 3 THEN 'sorting tuples'::text + WHEN 4 THEN 'writing new heap'::text + WHEN 5 THEN 'swapping relation files'::text + WHEN 6 THEN 'rebuilding index'::text + WHEN 7 THEN 'performing final cleanup'::text + ELSE NULL::text + END AS phase, + (s.param3)::oid AS repack_index_relid, + s.param4 AS heap_tuples_scanned, + s.param5 AS heap_tuples_written, + s.param6 AS heap_blks_total, + s.param7 AS heap_blks_scanned, + s.param8 AS index_rebuild_count + FROM (pg_stat_get_progress_info('REPACK'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20) + LEFT JOIN pg_database d ON ((s.datid = d.oid))); pg_stat_progress_vacuum| SELECT s.pid, s.datid, d.datname, diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index b7115f86104..cfcc3dc9761 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -76,6 +76,19 @@ INSERT INTO clstr_tst (b, c) VALUES (1111, 'this should fail'); SELECT conname FROM pg_constraint WHERE conrelid = 'clstr_tst'::regclass ORDER BY 1; +-- REPACK handles individual tables identically to CLUSTER, but it's worth +-- checking if it handles table hierarchies identically as well. +REPACK clstr_tst USING INDEX clstr_tst_c; + +-- Verify that inheritance link still works +INSERT INTO clstr_tst_inh VALUES (0, 100, 'in child table 2'); +SELECT a,b,c,substring(d for 30), length(d) from clstr_tst; + +-- Verify that foreign key link still works +INSERT INTO clstr_tst (b, c) VALUES (1111, 'this should fail'); + +SELECT conname FROM pg_constraint WHERE conrelid = 'clstr_tst'::regclass +ORDER BY 1; SELECT relname, relkind, EXISTS(SELECT 1 FROM pg_class WHERE oid = c.reltoastrelid) AS hastoast @@ -159,6 +172,34 @@ INSERT INTO clstr_1 VALUES (1); CLUSTER clstr_1; SELECT * FROM clstr_1; +-- REPACK w/o argument performs no ordering, so we can only check which tables +-- have the relfilenode changed. +RESET SESSION AUTHORIZATION; +CREATE TEMP TABLE relnodes_old AS +(SELECT relname, relfilenode +FROM pg_class +WHERE relname IN ('clstr_1', 'clstr_2', 'clstr_3')); + +SET SESSION AUTHORIZATION regress_clstr_user; +SET client_min_messages = ERROR; -- order of "skipping" warnings may vary +REPACK; +RESET client_min_messages; + +RESET SESSION AUTHORIZATION; +CREATE TEMP TABLE relnodes_new AS +(SELECT relname, relfilenode +FROM pg_class +WHERE relname IN ('clstr_1', 'clstr_2', 'clstr_3')); + +-- Do the actual comparison. Unlike CLUSTER, clstr_3 should have been +-- processed because there is nothing like clustering index here. +SELECT o.relname FROM relnodes_old o +JOIN relnodes_new n ON o.relname = n.relname +WHERE o.relfilenode <> n.relfilenode +ORDER BY o.relname; + +SET SESSION AUTHORIZATION regress_clstr_user; + -- Test MVCC-safety of cluster. There isn't much we can do to verify the -- results with a single backend... @@ -229,6 +270,24 @@ SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM o CLUSTER clstrpart; ALTER TABLE clstrpart SET WITHOUT CLUSTER; ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; + +-- Check that REPACK sets new relfilenodes: it should process exactly the same +-- tables as CLUSTER did. +DROP TABLE old_cluster_info; +DROP TABLE new_cluster_info; +CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +REPACK clstrpart USING INDEX clstrpart_idx; +CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C"; + +-- And finally the same for REPACK w/o index. +DROP TABLE old_cluster_info; +DROP TABLE new_cluster_info; +CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +REPACK clstrpart; +CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; +SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C"; + DROP TABLE clstrpart; -- Ownership of partitions is checked diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index c3f05796a7c..fbfd875b5c2 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -422,6 +422,7 @@ ClientCertName ClientConnectionInfo ClientData ClientSocket +ClusterCommand ClonePtrType ClosePortalStmt ClosePtrType @@ -2515,6 +2516,7 @@ ReorderBufferTupleCidKey ReorderBufferUpdateProgressTxnCB ReorderTuple RepOriginId +RepackStmt ReparameterizeForeignPathByChild_function ReplaceVarsFromTargetList_context ReplaceVarsNoMatchOption -- 2.43.5