public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v7 2/7] Implement CLUSTER of partitioned table.. 3+ messages / 3 participants [nested] [flat]
* [PATCH v7 2/7] Implement CLUSTER of partitioned table.. @ 2020-06-07 21:58 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-06-07 21:58 UTC (permalink / raw) This requires either specification of a partitioned index on which to cluster, or that an partitioned index was previously set clustered. --- doc/src/sgml/ref/cluster.sgml | 6 + src/backend/commands/cluster.c | 174 +++++++++++++++++++------- src/bin/psql/tab-complete.c | 1 + src/include/commands/cluster.h | 1 + src/test/regress/expected/cluster.out | 58 ++++++++- src/test/regress/sql/cluster.sql | 24 +++- 6 files changed, 209 insertions(+), 55 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 5dd21a0189..fb5deddb35 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -192,6 +192,12 @@ CLUSTER [VERBOSE] are periodically reclustered. </para> + <para> + Clustering a partitioned table clusters each of its partitions using the + index partition of the given partitioned index or (if not specified) the + partitioned index marked as clustered. + </para> + </refsect1> <refsect1> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 096a06f7b3..9b6673867c 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -32,7 +32,9 @@ #include "catalog/index.h" #include "catalog/namespace.h" #include "catalog/objectaccess.h" +#include "catalog/partition.h" #include "catalog/pg_am.h" +#include "catalog/pg_inherits.h" #include "catalog/toasting.h" #include "commands/cluster.h" #include "commands/defrem.h" @@ -73,6 +75,9 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); static List *get_tables_to_cluster(MemoryContext cluster_context); +static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context, + Oid indexOid); +static void cluster_multiple_rels(List *rvs, int options); /*--------------------------------------------------------------------------- @@ -135,7 +140,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) AccessExclusiveLock, 0, RangeVarCallbackOwnsTable, NULL); - rel = table_open(tableOid, NoLock); + rel = table_open(tableOid, ShareUpdateExclusiveLock); /* * Reject clustering a remote temp table ... their local buffer @@ -146,14 +151,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot cluster temporary tables of other sessions"))); - /* - * Reject clustering a partitioned table. - */ - if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot cluster a partitioned table"))); - if (stmt->indexname == NULL) { ListCell *index; @@ -189,10 +186,34 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) } /* close relation, keep lock till commit */ - table_close(rel, NoLock); + table_close(rel, ShareUpdateExclusiveLock); - /* Do the job. */ - cluster_rel(tableOid, indexOid, ¶ms); + if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + { + /* Do the job. */ + cluster_rel(tableOid, indexOid, ¶ms); + } + else + { + List *rvs; + MemoryContext cluster_context; + + /* Refuse to hold strong locks in a user transaction */ + PreventInTransactionBlock(isTopLevel, "CLUSTER"); + + cluster_context = AllocSetContextCreate(PortalContext, + "Cluster", + ALLOCSET_DEFAULT_SIZES); + + rvs = get_tables_to_cluster_partitioned(cluster_context, indexOid); + cluster_multiple_rels(rvs, params.options); + + /* Start a new transaction for the cleanup work. */ + StartTransactionCommand(); + + /* Clean up working storage */ + MemoryContextDelete(cluster_context); + } } else { @@ -202,7 +223,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) */ MemoryContext cluster_context; List *rvs; - ListCell *rv; /* * We cannot run this form of CLUSTER inside a user transaction block; @@ -225,28 +245,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) * cluster_context. */ rvs = get_tables_to_cluster(cluster_context); - - /* Commit to get out of starting transaction */ - PopActiveSnapshot(); - CommitTransactionCommand(); - - /* Ok, now that we've got them all, cluster them one by one */ - foreach(rv, rvs) - { - RelToCluster *rvtc = (RelToCluster *) lfirst(rv); - ClusterParams cluster_params = params; - - /* Start a new transaction for each relation. */ - StartTransactionCommand(); - /* functions in indexes may want a snapshot set */ - PushActiveSnapshot(GetTransactionSnapshot()); - /* Do the job. */ - cluster_params.options |= CLUOPT_RECHECK; - cluster_rel(rvtc->tableOid, rvtc->indexOid, - &cluster_params); - PopActiveSnapshot(); - CommitTransactionCommand(); - } + cluster_multiple_rels(rvs, params.options | CLUOPT_RECHECK_ISCLUSTERED); /* Start a new transaction for the cleanup work. */ StartTransactionCommand(); @@ -352,9 +351,10 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) } /* - * Check that the index is still the one with indisclustered set. + * Check that the index is still the one with indisclustered set, if needed. */ - if (!get_index_isclustered(indexOid)) + if ((params->options & CLUOPT_RECHECK_ISCLUSTERED) != 0 && + !get_index_isclustered(indexOid)) { relation_close(OldHeap, AccessExclusiveLock); pgstat_progress_end_command(); @@ -398,8 +398,13 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) /* Check heap and index are valid to cluster on */ if (OidIsValid(indexOid)) + { check_index_is_clusterable(OldHeap, indexOid, recheck, AccessExclusiveLock); + /* Mark the index as clustered */ + mark_index_clustered(OldHeap, indexOid, true); + } + /* * Quietly ignore the request if this is a materialized view which has not * been populated from its query. No harm is done because there is no data @@ -415,6 +420,14 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) return; } + /* For a partitioned rel, we're done. */ + if (!RELKIND_HAS_STORAGE(get_rel_relkind(tableOid))) + { + relation_close(OldHeap, AccessExclusiveLock); + pgstat_progress_end_command(); + return; + } + /* * All predicate locks on the tuples or pages are about to be made * invalid, because we move tuples around. Promote them to relation @@ -483,6 +496,9 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD * the worst consequence of following broken HOT chains would be that we * might put recently-dead tuples out-of-order in the new table, and there * is little harm in that.) + * + * This also refuses to cluster on an "incomplete" partitioned index + * created with "ON ONLY". */ if (!OldIndex->rd_index->indisvalid) ereport(ERROR, @@ -507,12 +523,6 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) Relation pg_index; ListCell *index; - /* Disallow applying to a partitioned table */ - if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot mark index clustered in partitioned table"))); - /* * If the index is already marked clustered, no need to do anything. */ @@ -584,10 +594,6 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) TransactionId frozenXid; MultiXactId cutoffMulti; - /* Mark the correct index as clustered */ - if (OidIsValid(indexOid)) - mark_index_clustered(OldHeap, indexOid, true); - /* Remember info about rel before closing OldHeap */ relpersistence = OldHeap->rd_rel->relpersistence; is_system_catalog = IsSystemRelation(OldHeap); @@ -1582,3 +1588,77 @@ get_tables_to_cluster(MemoryContext cluster_context) return rvs; } + +/* + * Return a List of tables and associated index, where each index is a + * partition of the given index + */ +static List * +get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) +{ + List *inhoids; + ListCell *lc; + List *rvs = NIL; + MemoryContext old_context; + + inhoids = find_all_inheritors(indexOid, NoLock, NULL); + + foreach(lc, inhoids) + { + Oid indexrelid = lfirst_oid(lc); + Oid relid = IndexGetRelation(indexrelid, false); + RelToCluster *rvtc; + + /* + * Partitioned rels are also processed by cluster_rel, to + * call check_index_is_clusterable() and mark_index_clustered(). + */ + + /* + * We have to build the list in a different memory context so it will + * survive the cross-transaction processing + */ + old_context = MemoryContextSwitchTo(cluster_context); + + rvtc = (RelToCluster *) palloc(sizeof(RelToCluster)); + rvtc->tableOid = relid; + rvtc->indexOid = indexrelid; + rvs = lappend(rvs, rvtc); + + MemoryContextSwitchTo(old_context); + } + + return rvs; +} + +/* Cluster each relation in a separate transaction */ +static void +cluster_multiple_rels(List *rvs, int options) +{ + ListCell *lc; + + /* Commit to get out of starting transaction */ + PopActiveSnapshot(); + CommitTransactionCommand(); + + /* Ok, now that we've got them all, cluster them one by one */ + foreach(lc, rvs) + { + RelToCluster *rvtc = (RelToCluster *) lfirst(lc); + ClusterParams cluster_params = { .options = options, }; + + /* Start a new transaction for each relation. */ + StartTransactionCommand(); + + /* functions in indexes may want a snapshot set */ + PushActiveSnapshot(GetTransactionSnapshot()); + + /* Do the job. */ + cluster_params.options |= CLUOPT_RECHECK; + cluster_rel(rvtc->tableOid, rvtc->indexOid, + &cluster_params); + + PopActiveSnapshot(); + CommitTransactionCommand(); + } +} diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index a75647b1cc..781aa95abc 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -588,6 +588,7 @@ static const SchemaQuery Query_for_list_of_clusterables = { .catname = "pg_catalog.pg_class c", .selcondition = "c.relkind IN (" CppAsString2(RELKIND_RELATION) ", " + CppAsString2(RELKIND_PARTITIONED_TABLE) ", " CppAsString2(RELKIND_MATVIEW) ")", .viscondition = "pg_catalog.pg_table_is_visible(c.oid)", .namespace = "c.relnamespace", diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index a941f2accd..c30ca01726 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -22,6 +22,7 @@ /* flag bits for ClusterParams->flags */ #define CLUOPT_RECHECK 0x01 /* recheck relation state */ #define CLUOPT_VERBOSE 0x02 /* print progress info */ +#define CLUOPT_RECHECK_ISCLUSTERED 0x04 /* recheck relation state for indisclustered */ /* options for CLUSTER */ typedef struct ClusterParams diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index bdae8fe00c..c74cfa88cc 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -439,14 +439,62 @@ select * from clstr_temp; drop table clstr_temp; RESET SESSION AUTHORIZATION; --- Check that partitioned tables cannot be clustered +-- Check that partitioned tables can be clustered CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a); +CREATE TABLE clstrpart1 PARTITION OF clstrpart FOR VALUES FROM (1)TO(10) PARTITION BY RANGE (a); +CREATE TABLE clstrpart11 PARTITION OF clstrpart1 FOR VALUES FROM (1)TO(10); +CREATE TABLE clstrpart12 PARTITION OF clstrpart1 FOR VALUES FROM (10)TO(20) PARTITION BY RANGE(a); +CREATE TABLE clstrpart2 PARTITION OF clstrpart FOR VALUES FROM (20)TO(30); +CREATE TABLE clstrpart3 PARTITION OF clstrpart DEFAULT PARTITION BY RANGE(a); +CREATE TABLE clstrpart33 PARTITION OF clstrpart3 DEFAULT; +ALTER TABLE clstrpart SET WITHOUT CLUSTER; +CREATE INDEX clstrpart_only_idx ON ONLY clstrpart (a); +CLUSTER clstrpart USING clstrpart_only_idx; -- fails +ERROR: cannot cluster on invalid index "clstrpart_only_idx" +DROP INDEX clstrpart_only_idx; CREATE INDEX clstrpart_idx ON clstrpart (a); -ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; -ERROR: cannot mark index clustered in partitioned table +-- Check that clustering sets new relfilenodes: +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 ; CLUSTER clstrpart USING clstrpart_idx; -ERROR: cannot cluster a partitioned table -DROP TABLE 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) + +-- Check that clustering sets new indisclustered: +SELECT relname, relkind, indisclustered FROM pg_partition_tree('clstrpart_idx'::regclass) AS tree JOIN pg_index i ON i.indexrelid=tree.relid JOIN pg_class c ON c.oid=indexrelid ORDER BY relname COLLATE "C"; + relname | relkind | indisclustered +-------------------+---------+---------------- + clstrpart11_a_idx | i | t + clstrpart12_a_idx | I | t + clstrpart1_a_idx | I | t + clstrpart2_a_idx | i | t + clstrpart33_a_idx | i | t + clstrpart3_a_idx | I | t + clstrpart_idx | I | t +(7 rows) + +CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitioned +CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs +CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf +\d clstrpart + Partitioned table "public.clstrpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition key: RANGE (a) +Indexes: + "clstrpart_idx" btree (a) CLUSTER +Number of partitions: 3 (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 188183647c..9bcc77695c 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -196,12 +196,30 @@ drop table clstr_temp; RESET SESSION AUTHORIZATION; --- Check that partitioned tables cannot be clustered +-- Check that partitioned tables can be clustered CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a); +CREATE TABLE clstrpart1 PARTITION OF clstrpart FOR VALUES FROM (1)TO(10) PARTITION BY RANGE (a); +CREATE TABLE clstrpart11 PARTITION OF clstrpart1 FOR VALUES FROM (1)TO(10); +CREATE TABLE clstrpart12 PARTITION OF clstrpart1 FOR VALUES FROM (10)TO(20) PARTITION BY RANGE(a); +CREATE TABLE clstrpart2 PARTITION OF clstrpart FOR VALUES FROM (20)TO(30); +CREATE TABLE clstrpart3 PARTITION OF clstrpart DEFAULT PARTITION BY RANGE(a); +CREATE TABLE clstrpart33 PARTITION OF clstrpart3 DEFAULT; +ALTER TABLE clstrpart SET WITHOUT CLUSTER; +CREATE INDEX clstrpart_only_idx ON ONLY clstrpart (a); +CLUSTER clstrpart USING clstrpart_only_idx; -- fails +DROP INDEX clstrpart_only_idx; CREATE INDEX clstrpart_idx ON clstrpart (a); -ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +-- Check that clustering sets new relfilenodes: +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 ; CLUSTER clstrpart USING clstrpart_idx; -DROP TABLE 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"; +-- Check that clustering sets new indisclustered: +SELECT relname, relkind, indisclustered FROM pg_partition_tree('clstrpart_idx'::regclass) AS tree JOIN pg_index i ON i.indexrelid=tree.relid JOIN pg_class c ON c.oid=indexrelid ORDER BY relname COLLATE "C"; +CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitioned +CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs +CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf +\d clstrpart -- Test CLUSTER with external tuplesorting -- 2.17.0 --AA9g+nFNFPYNJKiL Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0003-Propagate-changes-to-indisclustered-to-child-pare.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: psql: \dl+ to list large objects privileges @ 2022-01-04 06:24 Justin Pryzby <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Justin Pryzby @ 2022-01-04 06:24 UTC (permalink / raw) To: Pavel Luzanov <[email protected]>; +Cc: [email protected] On Tue, Aug 31, 2021 at 05:14:12PM +0300, Pavel Luzanov wrote: > I decided to move the do_lo_list function to describe.c in order to use the > printACLColumn helper function. And at the same time I renamed do_lo_list to > listLargeObjects to unify with the names of other similar functions. The tabs were changed to spaces when you moved the function. I suggest to move the function in a separate 0001 commit, which makes no code changes other than moving from one file to another. A committer would probably push them as a single patch, but this makes it easier to read and review the changes in 0002. Possibly like git diff HEAD~:src/bin/psql/large_obj.c src/bin/psql/describe.c > + if (pset.sversion >= 90000) Since a few weeks ago, psql no longer supports server versions before 9.2, so the "if" branch can go away. > I don't like how I handled the + modifier in the \lo_list command. But I > don't know how to do better now. This is the second time I've programmed in > C. The first time was the 'Hello World' program. So maybe something is done > wrong. I think everywhere else just uses verbose = strchr(cmd, '+') != 0; -- Justin ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: psql: \dl+ to list large objects privileges @ 2022-01-04 20:42 Tom Lane <[email protected]> parent: Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2022-01-04 20:42 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Pavel Luzanov <[email protected]>; [email protected] Justin Pryzby <[email protected]> writes: > I suggest to move the function in a separate 0001 commit, which makes no code > changes other than moving from one file to another. > A committer would probably push them as a single patch, but this makes it > easier to read and review the changes in 0002. Yeah, I agree with that idea. It's really tough to notice small changes by hand when the entire code block has been moved somewhere else. > Since a few weeks ago, psql no longer supports server versions before 9.2, so > the "if" branch can go away. And, in fact, the patch is no longer applying per the cfbot, because that hasn't been done. To move things along a bit, I split the patch more or less as Justin suggests and brought it up to HEAD. I did not study the command.c changes, but the rest of it seems okay, with one exception: I don't like the test case much at all. For one thing, it will fail in the buildfarm because you didn't adhere to the convention that roles created by a regression test must be named regress_something. For another, there's considerable overlap with testing done in largeobject.sql, which already creates some commented large objects. That means there's an undesirable ordering dependency --- if somebody wanted to run largeobject.sql first, the expected output of psql.sql would change. I wonder if we shouldn't put these new tests in largeobject.sql instead. (That would mean there are two expected-files to change, which is a bit tedious, but it's not very hard.) regards, tom lane Attachments: [text/x-diff] 0001-move-and-rename-do_lo_list.patch (3.7K, ../../[email protected]/2-0001-move-and-rename-do_lo_list.patch) download | inline diff: diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index fb3bab9494..0d64757899 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -811,7 +811,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd) success = describeRoles(pattern, show_verbose, show_system); break; case 'l': - success = do_lo_list(); + success = listLargeObjects(); break; case 'L': success = listLanguages(pattern, show_verbose, show_system); @@ -1963,7 +1963,7 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd) } else if (strcmp(cmd + 3, "list") == 0) - success = do_lo_list(); + success = listLargeObjects(); else if (strcmp(cmd + 3, "unlink") == 0) { diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index c28788e84f..0d92f3a80b 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -6455,3 +6455,39 @@ listOpFamilyFunctions(const char *access_method_pattern, PQclear(res); return true; } + +/* + * \dl or \lo_list + * Lists large objects + */ +bool +listLargeObjects(void) +{ + PGresult *res; + char buf[1024]; + printQueryOpt myopt = pset.popt; + + snprintf(buf, sizeof(buf), + "SELECT oid as \"%s\",\n" + " pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n" + " pg_catalog.obj_description(oid, 'pg_largeobject') as \"%s\"\n" + " FROM pg_catalog.pg_largeobject_metadata " + " ORDER BY oid", + gettext_noop("ID"), + gettext_noop("Owner"), + gettext_noop("Description")); + + res = PSQLexec(buf); + if (!res) + return false; + + myopt.topt.tuples_only = false; + myopt.nullPrint = NULL; + myopt.title = _("Large objects"); + myopt.translate_header = true; + + printQuery(res, &myopt, pset.queryFout, false, pset.logfile); + + PQclear(res); + return true; +} diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h index 71b320f1fc..3a55e1e4ed 100644 --- a/src/bin/psql/describe.h +++ b/src/bin/psql/describe.h @@ -139,5 +139,7 @@ extern bool listOpFamilyOperators(const char *accessMethod_pattern, extern bool listOpFamilyFunctions(const char *access_method_pattern, const char *family_pattern, bool verbose); +/* \dl or \lo_list */ +extern bool listLargeObjects(void); #endif /* DESCRIBE_H */ diff --git a/src/bin/psql/large_obj.c b/src/bin/psql/large_obj.c index 10e47c87ac..243875be83 100644 --- a/src/bin/psql/large_obj.c +++ b/src/bin/psql/large_obj.c @@ -262,42 +262,3 @@ do_lo_unlink(const char *loid_arg) return true; } - - - -/* - * do_lo_list() - * - * Show all large objects in database with comments - */ -bool -do_lo_list(void) -{ - PGresult *res; - char buf[1024]; - printQueryOpt myopt = pset.popt; - - snprintf(buf, sizeof(buf), - "SELECT oid as \"%s\",\n" - " pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n" - " pg_catalog.obj_description(oid, 'pg_largeobject') as \"%s\"\n" - " FROM pg_catalog.pg_largeobject_metadata " - " ORDER BY oid", - gettext_noop("ID"), - gettext_noop("Owner"), - gettext_noop("Description")); - - res = PSQLexec(buf); - if (!res) - return false; - - myopt.topt.tuples_only = false; - myopt.nullPrint = NULL; - myopt.title = _("Large objects"); - myopt.translate_header = true; - - printQuery(res, &myopt, pset.queryFout, false, pset.logfile); - - PQclear(res); - return true; -} diff --git a/src/bin/psql/large_obj.h b/src/bin/psql/large_obj.h index 003acbf52c..3172a7704d 100644 --- a/src/bin/psql/large_obj.h +++ b/src/bin/psql/large_obj.h @@ -11,6 +11,5 @@ bool do_lo_export(const char *loid_arg, const char *filename_arg); bool do_lo_import(const char *filename_arg, const char *comment_arg); bool do_lo_unlink(const char *loid_arg); -bool do_lo_list(void); #endif /* LARGE_OBJ_H */ [text/x-diff] 0002-print-large-object-acls.patch (8.5K, ../../[email protected]/3-0002-print-large-object-acls.patch) download | inline diff: diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 64d9030652..22f6c5c7ab 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2146,7 +2146,7 @@ REVOKE ALL ON accounts FROM PUBLIC; <entry><literal>LARGE OBJECT</literal></entry> <entry><literal>rw</literal></entry> <entry>none</entry> - <entry></entry> + <entry><literal>\dl+</literal></entry> </row> <row> <entry><literal>SCHEMA</literal></entry> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index ae38d3dcc3..1ab200a4ad 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1681,11 +1681,14 @@ testdb=> <varlistentry> - <term><literal>\dl</literal></term> + <term><literal>\dl[+]</literal></term> <listitem> <para> This is an alias for <command>\lo_list</command>, which shows a list of large objects. + If <literal>+</literal> is appended to the command name, + each large object is listed with its associated permissions, + if any. </para> </listitem> </varlistentry> @@ -2610,12 +2613,15 @@ lo_import 152801 </varlistentry> <varlistentry> - <term><literal>\lo_list</literal></term> + <term><literal>\lo_list[+]</literal></term> <listitem> <para> Shows a list of all <productname>PostgreSQL</productname> large objects currently stored in the database, along with any comments provided for them. + If <literal>+</literal> is appended to the command name, + each large object is listed with its associated permissions, + if any. </para> </listitem> </varlistentry> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 0d64757899..ceedcc860c 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -811,7 +811,16 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd) success = describeRoles(pattern, show_verbose, show_system); break; case 'l': - success = listLargeObjects(); + switch (cmd[2]) + { + case '\0': + case '+': + success = listLargeObjects(show_verbose); + break; + default: + status = PSQL_CMD_UNKNOWN; + break; + } break; case 'L': success = listLanguages(pattern, show_verbose, show_system); @@ -1928,11 +1937,13 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd) { char *opt1, *opt2; + bool show_verbose; opt1 = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, true); opt2 = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, true); + show_verbose = strchr(cmd, '+') ? true : false; if (strcmp(cmd + 3, "export") == 0) { @@ -1962,8 +1973,8 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd) } } - else if (strcmp(cmd + 3, "list") == 0) - success = listLargeObjects(); + else if (strcmp(cmd + 3, "list") == 0 || strcmp(cmd + 3, "list+") == 0) + success = listLargeObjects(show_verbose); else if (strcmp(cmd + 3, "unlink") == 0) { diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 0d92f3a80b..5edab7f938 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -6461,27 +6461,37 @@ listOpFamilyFunctions(const char *access_method_pattern, * Lists large objects */ bool -listLargeObjects(void) +listLargeObjects(bool verbose) { + PQExpBufferData buf; PGresult *res; - char buf[1024]; printQueryOpt myopt = pset.popt; - snprintf(buf, sizeof(buf), - "SELECT oid as \"%s\",\n" - " pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n" - " pg_catalog.obj_description(oid, 'pg_largeobject') as \"%s\"\n" - " FROM pg_catalog.pg_largeobject_metadata " - " ORDER BY oid", - gettext_noop("ID"), - gettext_noop("Owner"), - gettext_noop("Description")); - - res = PSQLexec(buf); + initPQExpBuffer(&buf); + + printfPQExpBuffer(&buf, + "SELECT oid as \"%s\",\n" + " pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n ", + gettext_noop("ID"), + gettext_noop("Owner")); + + if (verbose) + { + printACLColumn(&buf, "lomacl"); + appendPQExpBufferStr(&buf, ",\n "); + } + + appendPQExpBuffer(&buf, + "pg_catalog.obj_description(oid, 'pg_largeobject') as \"%s\"\n" + "FROM pg_catalog.pg_largeobject_metadata\n" + "ORDER BY oid", + gettext_noop("Description")); + + res = PSQLexec(buf.data); + termPQExpBuffer(&buf); if (!res) return false; - myopt.topt.tuples_only = false; myopt.nullPrint = NULL; myopt.title = _("Large objects"); myopt.translate_header = true; diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h index 3a55e1e4ed..b57ba67bba 100644 --- a/src/bin/psql/describe.h +++ b/src/bin/psql/describe.h @@ -140,6 +140,6 @@ extern bool listOpFamilyFunctions(const char *access_method_pattern, const char *family_pattern, bool verbose); /* \dl or \lo_list */ -extern bool listLargeObjects(void); +extern bool listLargeObjects(bool verbose); #endif /* DESCRIBE_H */ diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index 8cadfbb103..5752a36ac8 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -248,7 +248,7 @@ slashUsage(unsigned short int pager) fprintf(output, _(" \\dFt[+] [PATTERN] list text search templates\n")); fprintf(output, _(" \\dg[S+] [PATTERN] list roles\n")); fprintf(output, _(" \\di[S+] [PATTERN] list indexes\n")); - fprintf(output, _(" \\dl list large objects, same as \\lo_list\n")); + fprintf(output, _(" \\dl[+] list large objects, same as \\lo_list\n")); fprintf(output, _(" \\dL[S+] [PATTERN] list procedural languages\n")); fprintf(output, _(" \\dm[S+] [PATTERN] list materialized views\n")); fprintf(output, _(" \\dn[S+] [PATTERN] list schemas\n")); @@ -325,7 +325,7 @@ slashUsage(unsigned short int pager) fprintf(output, _("Large Objects\n")); fprintf(output, _(" \\lo_export LOBOID FILE\n" " \\lo_import FILE [COMMENT]\n" - " \\lo_list\n" + " \\lo_list[+]\n" " \\lo_unlink LOBOID large object operations\n")); ClosePager(output); diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 6428ebc507..0aa98317a7 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -5290,3 +5290,36 @@ ERROR: relation "notexists" does not exist LINE 1: SELECT * FROM notexists; ^ STATEMENT: SELECT * FROM notexists; + lo_create +----------- + 42 +(1 row) + + Large objects + ID | Owner | Description +----+---------+--------------------------------------------- + 42 | lo_test | the answer to the ultimate question of life +(1 row) + + Large objects + ID | Owner | Access privileges | Description +----+---------+--------------------+--------------------------------------------- + 42 | lo_test | lo_test=rw/lo_test+| the answer to the ultimate question of life + | | =r/lo_test | +(1 row) + + Large objects + ID | Owner | Description +----+---------+--------------------------------------------- + 42 | lo_test | the answer to the ultimate question of life +(1 row) + + Large objects + ID | Owner | Access privileges | Description +----+---------+--------------------+--------------------------------------------- + 42 | lo_test | lo_test=rw/lo_test+| the answer to the ultimate question of life + | | =r/lo_test | +(1 row) + +invalid command \dl- +invalid command \lo_list- diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index d4e4fdbbb7..dea081f937 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -1316,3 +1316,19 @@ DROP TABLE oer_test; \set ECHO errors SELECT * FROM notexists; \set ECHO none + +-- check printing info about large objects +-- one large object with OID=42 and owner lo_test is expected in the output +CREATE ROLE lo_test; +SELECT lo_create(42); +ALTER LARGE OBJECT 42 OWNER TO lo_test; +COMMENT ON LARGE OBJECT 42 IS 'the answer to the ultimate question of life'; +GRANT SELECT ON LARGE OBJECT 42 TO public; +\dl +\dl+ +\lo_list +\lo_list+ +\dl- +\lo_list- +\lo_unlink 42 +DROP ROLE lo_test; ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-01-04 20:42 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-06-07 21:58 [PATCH v7 2/7] Implement CLUSTER of partitioned table.. Justin Pryzby <[email protected]> 2022-01-04 06:24 Re: psql: \dl+ to list large objects privileges Justin Pryzby <[email protected]> 2022-01-04 20:42 ` Re: psql: \dl+ to list large objects privileges Tom Lane <[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