| From: | Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Report buffer access strategies in EXPLAIN |
| Date: | 2026-10-28 21:21:49 |
| Message-ID: | CAN12+YLyKPTFhoJqAMt-2uAjkHVXD0qZuU0MZchB0Z-ADaDxWA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> I removed the rescan assertion and now retain the most recently reported strategy,
> as on the table AM side. Reporting mixed would be more precise if an extension
> changes strategy on rescan, but I preferred to keep it simple for now since in
> core AMs do not do that.
I think this is an important point for what the "Buffer Access
Strategy:" field means.
IMO we should make the rule as: normal does not overwrite an already-recorded
non-normal strategy.
There is a core case here too.
For example, I added this instrumentation:
```
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1327,6 +1327,10 @@ heap_beginscan(Relation relation, Snapshot snapshot,
{
scan->rs_base.rs_instrument->strategy =
GetAccessStrategyType(scan->rs_strategy);
scan->rs_base.rs_instrument->strategy_valid = true;
+ elog(NOTICE, "heap_beginscan strategy: scan=%p
rel=%s strategy=%d",
+ (void *) scan,
+ RelationGetRelationName(scan->rs_base.rs_rd),
+ scan->rs_base.rs_instrument->strategy);
}
/* read stream instrumentation */
@@ -1402,6 +1406,10 @@ heap_rescan(TableScanDesc sscan, ScanKey key,
bool set_params,
{
scan->rs_base.rs_instrument->strategy =
GetAccessStrategyType(scan->rs_strategy);
scan->rs_base.rs_instrument->strategy_valid = true;
+ elog(NOTICE, "heap_rescan strategy: scan=%p rel=%s strategy=%d",
+ (void *) scan,
+ RelationGetRelationName(scan->rs_base.rs_rd),
+ scan->rs_base.rs_instrument->strategy);
}
}
```
and ran this case in which a parameterized TABLESAMPLE SYSTEM scan can move
between normal and bulkread across rescans depending on the outer parameter
value.
```
postgres=# SET client_min_messages = notice;
DROP TABLE IF EXISTS sample_strategy_repro;
CREATE TABLE sample_strategy_repro AS
SELECT g AS i
FROM generate_series(1, 2200000) AS g;
ANALYZE sample_strategy_repro;
EXPLAIN (ANALYZE, BUFFERS)
SELECT count(*)
FROM (
SELECT p
FROM (VALUES (1, 100::real), (2, 0.5::real)) v(ord, p)
ORDER BY ord
) o
JOIN LATERAL (
SELECT *
FROM sample_strategy_repro TABLESAMPLE SYSTEM (o.p) REPEATABLE (0)
) s ON true;
SET
NOTICE: table "sample_strategy_repro" does not exist, skipping
DROP TABLE
SELECT 2200000
ANALYZE
NOTICE: heap_beginscan strategy: scan=0x1b2fa8c8
rel=sample_strategy_repro strategy=1
NOTICE: heap_rescan strategy: scan=0x1b2fa8c8
rel=sample_strategy_repro strategy=0
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------
Aggregate (cost=17732.06..17732.07 rows=1 width=8) (actual
time=1183.289..1183.292 rows=1.00 loops=1)
Buffers: shared hit=12081 read=7488 dirtied=7489 written=57
-> Nested Loop (cost=0.04..16632.06 rows=440000 width=0) (actual
time=0.353..963.297 rows=2210396.00 loops=1)
Buffers: shared hit=12081 read=7488 dirtied=7489 written=57
-> Sort (cost=0.04..0.04 rows=2 width=8) (actual
time=0.012..0.016 rows=2.00 loops=1)
Sort Key: "*VALUES*".column1
Sort Method: quicksort Memory: 25kB
-> Values Scan on "*VALUES*" (cost=0.00..0.03 rows=2
width=8) (actual time=0.003..0.005 rows=2.00 loops=1)
-> Sample Scan on sample_strategy_repro (cost=0.00..6116.00
rows=220000 width=0) (actual time=0.204..376.754 rows=1105198.00
loops=2)
Sampling: system ("*VALUES*".column2) REPEATABLE
('0'::double precision)
Buffer Access Strategy: normal
Buffers: shared hit=12081 read=7488 dirtied=7489 written=57
Planning:
Buffers: shared hit=87 read=7
Planning Time: 0.639 ms
Execution Time: 1183.341 ms
(16 rows)
```
I still do not think we need to show all the strategies used here, at least not
for the first version of this patch. What the user will care about most is
which ring strategy was used, so if anything other than normal was used, that
should take precedence. This should be documented.
I also have a few additional comments by patch:
v2-0002:
The glossary still reads as if all Buffer Access Strategies are ring-based, but
normal is listed below and explicitly does not use a strategy ring. I think this
wording would avoid that:
```
- A Buffer Access Strategy sets up references to a limited number of
- <glossterm linkend="glossary-shared-memory">shared buffers</glossterm> and
- reuses them circularly. When the operation requires a new page, a victim
- buffer is chosen from the buffers in the strategy ring, which may require
+ Except for <literal>normal</literal>, Buffer Access Strategies set up
+ references to a limited number of
+ <glossterm linkend="glossary-shared-memory">shared buffers</glossterm>
+ and reuse them circularly. When the operation requires a new page, a
+ victim buffer is chosen from the buffers in the strategy ring, which may
+ require
```
v2-0003:
In the pg_stat_io docs, I think the bulkwrite example should say COPY FROM
rather than COPY:
```
- strategy</glossterm>, such as <command>COPY</command>.
+ strategy</glossterm>, such as <command>COPY FROM</command>.
```
v2-0007:
I think the EXPLAIN docs should clarify that this field does not report
bulkwrite. For example, CTAS can use BAS_BULKWRITE for the target table, but
EXPLAIN only reports the source scan strategy here. I could not find an
EXPLAINable SQL command that would show bulkwrite through this field.
Maybe something like:
```
nodes report the <glossterm
linkend="glossary-buffer-access-strategy">buffer access
- strategy</glossterm> used by the table scan.
+ strategy</glossterm> used by the table scan. This field reports
+ strategies for scan nodes; it does not show write-side strategies such as
+ <literal>bulkwrite</literal>, even when a command uses them internally.
```
v2-0008:
bloom records BAS_BULKREAD in blrescan(), before it knows whether blgetbitmap()
will actually scan the index. With a generic prepared plan and a NULL parameter,
blgetbitmap() returns from the SK_ISNULL fast path, so EXPLAIN can show
"Buffer Access Strategy: bulkread" together with "Index Searches: 0".
For example:
```
CREATE EXTENSION IF NOT EXISTS bloom;
DROP TABLE IF EXISTS bloom_strategy_repro;
CREATE TABLE bloom_strategy_repro (i int);
INSERT INTO bloom_strategy_repro
SELECT generate_series(1, 1000);
CREATE INDEX bloom_strategy_repro_idx
ON bloom_strategy_repro USING bloom (i);
ANALYZE bloom_strategy_repro;
SET enable_seqscan = off;
SET plan_cache_mode = force_generic_plan;
PREPARE bloom_q(int) AS
SELECT *
FROM bloom_strategy_repro
WHERE i = $1;
EXPLAIN (ANALYZE, BUFFERS)
EXECUTE bloom_q(NULL);
```
Before the fix, the Bitmap Index Scan reports both:
```
Buffer Access Strategy: bulkread
Index Searches: 0
```
I think InstrRecordIndexScanStrategy(..., BAS_BULKREAD) should move to
blgetbitmap(), just before GetAccessStrategy(BAS_BULKREAD), after the NULL-key
early return:
```
@@ blrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- InstrRecordIndexScanStrategy(scan->instrument, BAS_BULKREAD);
-
if (so->sign)
pfree(so->sign);
@@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
/*
* We're going to read the whole index. This is why we use appropriate
* buffer access strategy.
*/
+ InstrRecordIndexScanStrategy(scan->instrument, BAS_BULKREAD);
bas = GetAccessStrategy(BAS_BULKREAD);
```
Maybe as a follow-up, but showing the ring size too seems useful also.
--
Sami
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Iliia Khaprov | 2026-10-28 21:31:53 | inet/cidr: IPv4 prefix length overflows int and passes the range check |
| Previous Message | surya poondla | 2026-10-28 20:42:45 | Re: Event trigger table rewrite doc missing ALTER MATERIALIZED VIEW case |