public inbox for [email protected]
help / color / mirror / Atom feedRe: [PATCH] allow pg_current_logfile() execution under pg_monitor role
8+ messages / 5 participants
[nested] [flat]
* Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-12 15:49 Nathan Bossart <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Bossart @ 2024-02-12 15:49 UTC (permalink / raw)
To: Pavlo Golub <[email protected]>; +Cc: pgsql-hackers
On Mon, Feb 12, 2024 at 12:27:54PM +0000, Pavlo Golub wrote:
>> Are there any other
>> functions that pg_monitor ought to have privileges for?
>>
> Not that I'm aware of at the moment. This one was found by chance.
Okay. I'll plan on committing this in the next few days.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-13 21:29 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Nathan Bossart @ 2024-02-13 21:29 UTC (permalink / raw)
To: Pavlo Golub <[email protected]>; +Cc: pgsql-hackers
On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
> Okay. I'll plan on committing this in the next few days.
Here is what I have staged for commit.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v2-0001-Allow-pg_monitor-to-execute-pg_current_logfile.patch (4.2K, ../../20240213212928.GA262402@nathanxps13/2-v2-0001-Allow-pg_monitor-to-execute-pg_current_logfile.patch)
download | inline diff:
From bfe542c5d7b3c981e75ac6551abb34fbdf646eea Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 13 Feb 2024 15:12:36 -0600
Subject: [PATCH v2 1/1] Allow pg_monitor to execute pg_current_logfile().
We allow roles with privileges of pg_monitor to execute functions
like pg_ls_logdir(), so it seems natural that such roles would also
be able to execute this function.
Bumps catversion.
Co-authored-by: Pavlo Golub
Discussion: https://postgr.es/m/CAK7ymcLmEYWyQkiCZ64WC-HCzXAB0omM%3DYpj9B3rXe8vUAFMqw%40mail.gmail.com
---
doc/src/sgml/func.sgml | 5 +++++
src/backend/catalog/system_functions.sql | 4 ++++
src/include/catalog/catversion.h | 2 +-
src/test/regress/expected/misc_functions.out | 20 ++++++++++++++++++++
src/test/regress/sql/misc_functions.sql | 11 +++++++++++
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 11d537b341..c4e5b4967e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23735,6 +23735,11 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
<xref linkend="guc-log-destination"/>.
The result reflects the contents of
the <filename>current_logfiles</filename> file.
+ </para>
+ <para>
+ This function is restricted to superusers and roles with privileges of
+ the <literal>pg_monitor</literal> role by default, but other users can
+ be granted EXECUTE to run the function.
</para></entry>
</row>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 346cfb98a0..fe2bb50f46 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -777,6 +777,10 @@ GRANT EXECUTE ON FUNCTION pg_ls_logicalmapdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_replslotdir(text) TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_current_logfile() TO pg_monitor;
+
+GRANT EXECUTE ON FUNCTION pg_current_logfile(text) TO pg_monitor;
+
GRANT pg_read_all_settings TO pg_monitor;
GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9fc8ac9290..80a4c19565 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202401301
+#define CATALOG_VERSION_NO 202402131
#endif
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 7c15477104..d5f61dfad9 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -683,3 +683,23 @@ SELECT gist_stratnum_identity(18::smallint);
18
(1 row)
+-- pg_current_logfile
+CREATE ROLE regress_current_logfile;
+-- not available by default
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+ has_function_privilege
+------------------------
+ f
+(1 row)
+
+GRANT pg_monitor TO regress_current_logfile;
+-- role has privileges of pg_monitor and can execute the function
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+ has_function_privilege
+------------------------
+ t
+(1 row)
+
+DROP ROLE regress_current_logfile;
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 851dad90f4..928b04db7f 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -254,3 +254,14 @@ FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size - 1),
-- test stratnum support functions
SELECT gist_stratnum_identity(3::smallint);
SELECT gist_stratnum_identity(18::smallint);
+
+-- pg_current_logfile
+CREATE ROLE regress_current_logfile;
+-- not available by default
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+GRANT pg_monitor TO regress_current_logfile;
+-- role has privileges of pg_monitor and can execute the function
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+DROP ROLE regress_current_logfile;
--
2.25.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-14 07:59 Daniel Gustafsson <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Daniel Gustafsson @ 2024-02-14 07:59 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Pavlo Golub <[email protected]>; pgsql-hackers
> On 13 Feb 2024, at 22:29, Nathan Bossart <[email protected]> wrote:
>
> On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
>> Okay. I'll plan on committing this in the next few days.
>
> Here is what I have staged for commit.
LGTM.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-14 17:50 Nathan Bossart <[email protected]>
parent: Daniel Gustafsson <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Nathan Bossart @ 2024-02-14 17:50 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Pavlo Golub <[email protected]>; pgsql-hackers
On Wed, Feb 14, 2024 at 08:59:06AM +0100, Daniel Gustafsson wrote:
> LGTM.
Committed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-14 18:05 Pavlo Golub <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Pavlo Golub @ 2024-02-14 18:05 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: pgsql-hackers
>On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
>> Okay. I'll plan on committing this in the next few days.
>
>Here is what I have staged for commit.
>
Oh, thanks! I forgot, indeed, to update docs and catalog version! My
bad!
In my defense, I was trying to find tests but I missed
regress/sql/misc_functions.sql somehow.
Now I will know. Thanks again!
Best regards,
Pavlo Golub
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-14 18:45 Tom Lane <[email protected]>
parent: Pavlo Golub <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2024-02-14 18:45 UTC (permalink / raw)
To: Pavlo Golub <[email protected]>; +Cc: Nathan Bossart <[email protected]>; pgsql-hackers
"Pavlo Golub" <[email protected]> writes:
> Oh, thanks! I forgot, indeed, to update docs and catalog version! My
> bad!
Docs, yes, but don't include catversion bumps in submitted patches.
They'll just lead to merge problems when somebody else changes the
current catversion. We rely on the committer to remember to do this
(which is an imperfect system, but...)
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role
@ 2024-02-14 18:47 Pavlo Golub <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Pavlo Golub @ 2024-02-14 18:47 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; pgsql-hackers
On Wed, Feb 14, 2024, 19:45 Tom Lane <[email protected]> wrote:
> "Pavlo Golub" <[email protected]> writes:
> > Oh, thanks! I forgot, indeed, to update docs and catalog version! My
> > bad!
>
> Docs, yes, but don't include catversion bumps in submitted patches.
> They'll just lead to merge problems when somebody else changes the
> current catversion. We rely on the committer to remember to do this
> (which is an imperfect system, but...)
>
Thanks for the clarification.
> regards, tom lane
>
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a6b98fa12a1..b2397fe2054 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2024-02-27 01:34 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-12 15:49 Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role Nathan Bossart <[email protected]>
2024-02-13 21:29 ` Nathan Bossart <[email protected]>
2024-02-14 07:59 ` Daniel Gustafsson <[email protected]>
2024-02-14 17:50 ` Nathan Bossart <[email protected]>
2024-02-14 18:05 ` Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role Pavlo Golub <[email protected]>
2024-02-14 18:45 ` Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role Tom Lane <[email protected]>
2024-02-14 18:47 ` Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role Pavlo Golub <[email protected]>
2024-02-27 01:34 [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[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