public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 3/6] Be less verbose on variable names 23+ messages / 4 participants [nested] [flat]
* [PATCH 3/6] Be less verbose on variable names @ 2019-11-28 22:30 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Alvaro Herrera @ 2019-11-28 22:30 UTC (permalink / raw) --- src/backend/commands/typecmds.c | 23 +++++++++------------- src/backend/utils/adt/pg_upgrade_support.c | 4 ++-- src/include/catalog/binary_upgrade.h | 4 ++-- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 1b012c9cad..4ca2d3364b 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -85,8 +85,8 @@ typedef struct /* Potentially set by pg_upgrade_support functions */ Oid binary_upgrade_next_array_pg_type_oid = InvalidOid; -Oid binary_upgrade_next_multirange_pg_type_oid = InvalidOid; -Oid binary_upgrade_next_multirange_array_pg_type_oid = InvalidOid; +Oid binary_upgrade_next_mrng_pg_type_oid = InvalidOid; +Oid binary_upgrade_next_mrng_array_pg_type_oid = InvalidOid; static void makeRangeConstructors(const char *name, Oid namespace, Oid rangeOid, Oid subtype); @@ -1529,13 +1529,9 @@ DefineRange(CreateRangeStmt *stmt) /* alignment must be 'i' or 'd' for ranges */ alignment = (subtypalign == 'd') ? 'd' : 'i'; - /* Allocate OID for array type */ + /* Allocate OID for array type, its multirange and its multirange array */ rangeArrayOid = AssignTypeArrayOid(); - - /* Allocate OID for multirange type */ multirangeOid = AssignTypeMultirangeOid(); - - /* Allocate OID for multirange array type */ multirangeArrayOid = AssignTypeMultirangeArrayOid(); /* Create the pg_type entry */ @@ -1574,7 +1570,6 @@ DefineRange(CreateRangeStmt *stmt) Assert(typoid == address.objectId); /* Create the multirange that goes with it */ - multirangeTypeName = makeMultirangeTypeName(typeName, typeNamespace); mltrngaddress = @@ -2313,13 +2308,13 @@ AssignTypeMultirangeOid(void) /* Use binary-upgrade override for pg_type.oid? */ if (IsBinaryUpgrade) { - if (!OidIsValid(binary_upgrade_next_multirange_pg_type_oid)) + if (!OidIsValid(binary_upgrade_next_mrng_pg_type_oid)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("pg_type multirange OID value not set when in binary upgrade mode"))); - type_multirange_oid = binary_upgrade_next_multirange_pg_type_oid; - binary_upgrade_next_multirange_pg_type_oid = InvalidOid; + type_multirange_oid = binary_upgrade_next_mrng_pg_type_oid; + binary_upgrade_next_mrng_pg_type_oid = InvalidOid; } else { @@ -2346,13 +2341,13 @@ AssignTypeMultirangeArrayOid(void) /* Use binary-upgrade override for pg_type.oid? */ if (IsBinaryUpgrade) { - if (!OidIsValid(binary_upgrade_next_multirange_array_pg_type_oid)) + if (!OidIsValid(binary_upgrade_next_mrng_array_pg_type_oid)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("pg_type multirange array OID value not set when in binary upgrade mode"))); - type_multirange_array_oid = binary_upgrade_next_multirange_array_pg_type_oid; - binary_upgrade_next_multirange_array_pg_type_oid = InvalidOid; + type_multirange_array_oid = binary_upgrade_next_mrng_array_pg_type_oid; + binary_upgrade_next_mrng_array_pg_type_oid = InvalidOid; } else { diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c index d980b96f48..418c26c81b 100644 --- a/src/backend/utils/adt/pg_upgrade_support.c +++ b/src/backend/utils/adt/pg_upgrade_support.c @@ -57,7 +57,7 @@ binary_upgrade_set_next_multirange_pg_type_oid(PG_FUNCTION_ARGS) Oid typoid = PG_GETARG_OID(0); CHECK_IS_BINARY_UPGRADE; - binary_upgrade_next_multirange_pg_type_oid = typoid; + binary_upgrade_next_mrng_pg_type_oid = typoid; PG_RETURN_VOID(); } @@ -68,7 +68,7 @@ binary_upgrade_set_next_multirange_array_pg_type_oid(PG_FUNCTION_ARGS) Oid typoid = PG_GETARG_OID(0); CHECK_IS_BINARY_UPGRADE; - binary_upgrade_next_multirange_array_pg_type_oid = typoid; + binary_upgrade_next_mrng_array_pg_type_oid = typoid; PG_RETURN_VOID(); } diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h index 2b6e87bb84..ba132ddf23 100644 --- a/src/include/catalog/binary_upgrade.h +++ b/src/include/catalog/binary_upgrade.h @@ -16,8 +16,8 @@ extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid; extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid; -extern PGDLLIMPORT Oid binary_upgrade_next_multirange_pg_type_oid; -extern PGDLLIMPORT Oid binary_upgrade_next_multirange_array_pg_type_oid; +extern PGDLLIMPORT Oid binary_upgrade_next_mrng_pg_type_oid; +extern PGDLLIMPORT Oid binary_upgrade_next_mrng_array_pg_type_oid; extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid; extern PGDLLIMPORT Oid binary_upgrade_next_heap_pg_class_oid; -- 2.20.1 --+QahgC5+KEYLbs62 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0004-Protect-comment-against-pgindent.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations @ 2022-02-04 19:45 Robert Haas <[email protected]> 2022-02-04 20:30 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Peter Geoghegan <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Robert Haas @ 2022-02-04 19:45 UTC (permalink / raw) To: Peter Geoghegan <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]> On Sat, Jan 29, 2022 at 11:43 PM Peter Geoghegan <[email protected]> wrote: > When VACUUM sees that all remaining/unpruned tuples on a page are > all-visible, it isn't just important because of cost control > considerations. It's deeper than that. It's also treated as a > tentative signal from the application itself, about the data itself. > Which is: this page looks "settled" -- it may never be updated again, > but if there is an update it likely won't change too much about the > whole page. While I agree that there's some case to be made for leaving settled pages well enough alone, your criterion for settled seems pretty much accidental. Imagine a system where there are two applications running, A and B. Application A runs all the time and all the transactions which it performs are short. Therefore, when a certain page is not modified by transaction A for a short period of time, the page will become all-visible and will be considered settled. Application B runs once a month and performs various transactions all of which are long, perhaps on a completely separate set of tables. While application B is running, pages take longer to settle not only for application B but also for application A. It doesn't make sense to say that the application is in control of the behavior when, in reality, it may be some completely separate application that is controlling the behavior. > The application is in charge, really -- not VACUUM. This is already > the case, whether we like it or not. VACUUM needs to learn to live in > that reality, rather than fighting it. When VACUUM considers a page > settled, and the physical page still has a relatively large amount of > free space (say 45% of BLCKSZ, a borderline case in the new FSM > patch), "losing" so much free space certainly is unappealing. We set > the free space to 0 in the free space map all the same, because we're > cutting our losses at that point. While the exact threshold I've > proposed is tentative, the underlying theory seems pretty sound to me. > The BLCKSZ/2 cutoff (and the way that it extends the general rules for > whole-page freezing) is intended to catch pages that are qualitatively > different, as well as quantitatively different. It is a balancing act, > between not wasting space, and the risk of systemic problems involving > excessive amounts of non-HOT updates that must move a successor > version to another page. I can see that this could have significant advantages under some circumstances. But I think it could easily be far worse under other circumstances. I mean, you can have workloads where you do some amount of read-write work on a table and then go read only and sequential scan it an infinite number of times. An algorithm that causes the table to be smaller at the point where we switch to read-only operations, even by a modest amount, wins infinitely over anything else. But even if you have no change in the access pattern, is it a good idea to allow the table to be, say, 5% larger if it means that correlated data is colocated? In general, probably yes. If that means that the table fails to fit in shared_buffers instead of fitting, no. If that means that the table fails to fit in the OS cache instead of fitting, definitely no. And to me, that kind of effect is why it's hard to gain much confidence in regards to stuff like this via laboratory testing. I mean, I'm glad you're doing such tests. But in a laboratory test, you tend not to have things like a sudden and complete change in the workload, or a random other application sometimes sharing the machine, or only being on the edge of running out of memory. I think in general people tend to avoid such things in benchmarking scenarios, but even if include stuff like this, it's hard to know what to include that would be representative of real life, because just about anything *could* happen in real life. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations 2022-02-04 19:45 Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> @ 2022-02-04 20:30 ` Peter Geoghegan <[email protected]> 2022-02-04 21:18 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Peter Geoghegan @ 2022-02-04 20:30 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]> On Fri, Feb 4, 2022 at 2:45 PM Robert Haas <[email protected]> wrote: > While I agree that there's some case to be made for leaving settled > pages well enough alone, your criterion for settled seems pretty much > accidental. I fully admit that I came up with the FSM heuristic with TPC-C in mind. But you have to start somewhere. Fortunately, the main benefit of this patch series (avoiding the freeze cliff during anti-wraparound VACUUMs, often avoiding anti-wraparound VACUUMs altogether) don't depend on the experimental FSM patch at all. I chose to post that now because it seemed to help with my more general point about qualitatively different pages, and freezing at the page level. > Imagine a system where there are two applications running, > A and B. Application A runs all the time and all the transactions > which it performs are short. Therefore, when a certain page is not > modified by transaction A for a short period of time, the page will > become all-visible and will be considered settled. Application B runs > once a month and performs various transactions all of which are long, > perhaps on a completely separate set of tables. While application B is > running, pages take longer to settle not only for application B but > also for application A. It doesn't make sense to say that the > application is in control of the behavior when, in reality, it may be > some completely separate application that is controlling the behavior. Application B will already block pruning by VACUUM operations against application A's table, and so effectively blocks recording of the resultant free space in the FSM in your scenario. And so application A and application B should be considered the same application already. That's just how VACUUM works. VACUUM isn't a passive observer of the system -- it's another participant. It both influences and is influenced by almost everything else in the system. > I can see that this could have significant advantages under some > circumstances. But I think it could easily be far worse under other > circumstances. I mean, you can have workloads where you do some amount > of read-write work on a table and then go read only and sequential > scan it an infinite number of times. An algorithm that causes the > table to be smaller at the point where we switch to read-only > operations, even by a modest amount, wins infinitely over anything > else. But even if you have no change in the access pattern, is it a > good idea to allow the table to be, say, 5% larger if it means that > correlated data is colocated? In general, probably yes. If that means > that the table fails to fit in shared_buffers instead of fitting, no. > If that means that the table fails to fit in the OS cache instead of > fitting, definitely no. 5% larger seems like a lot more than would be typical, based on what I've seen. I don't think that the regression in this scenario can be characterized as "infinitely worse", or anything like it. On a long enough timeline, the potential upside of something like this is nearly unlimited -- it could avoid a huge amount of write amplification. But the potential downside seems to be small and fixed -- which is the point (bounding the downside). The mere possibility of getting that big benefit (avoiding the costs from heap fragmentation) is itself a benefit, even when it turns out not to pay off in your particular case. It can be seen as insurance. > And to me, that kind of effect is why it's hard to gain much > confidence in regards to stuff like this via laboratory testing. I > mean, I'm glad you're doing such tests. But in a laboratory test, you > tend not to have things like a sudden and complete change in the > workload, or a random other application sometimes sharing the machine, > or only being on the edge of running out of memory. I think in general > people tend to avoid such things in benchmarking scenarios, but even > if include stuff like this, it's hard to know what to include that > would be representative of real life, because just about anything > *could* happen in real life. Then what could you have confidence in? -- Peter Geoghegan ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations 2022-02-04 19:45 Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> 2022-02-04 20:30 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Peter Geoghegan <[email protected]> @ 2022-02-04 21:18 ` Robert Haas <[email protected]> 2022-02-04 22:26 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Peter Geoghegan <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Robert Haas @ 2022-02-04 21:18 UTC (permalink / raw) To: Peter Geoghegan <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]> On Fri, Feb 4, 2022 at 3:31 PM Peter Geoghegan <[email protected]> wrote: > Application B will already block pruning by VACUUM operations against > application A's table, and so effectively blocks recording of the > resultant free space in the FSM in your scenario. And so application A > and application B should be considered the same application already. > That's just how VACUUM works. Sure ... but that also sucks. If we consider application A and application B to be the same application, then we're basing our decision about what to do on information that is inaccurate. > 5% larger seems like a lot more than would be typical, based on what > I've seen. I don't think that the regression in this scenario can be > characterized as "infinitely worse", or anything like it. On a long > enough timeline, the potential upside of something like this is nearly > unlimited -- it could avoid a huge amount of write amplification. But > the potential downside seems to be small and fixed -- which is the > point (bounding the downside). The mere possibility of getting that > big benefit (avoiding the costs from heap fragmentation) is itself a > benefit, even when it turns out not to pay off in your particular > case. It can be seen as insurance. I don't see it that way. There are cases where avoiding writes is better, and cases where trying to cram everything into the fewest possible ages is better. With the right test case you can make either strategy look superior. What I think your test case has going for it is that it is similar to something that a lot of people, really a ton of people, actually do with PostgreSQL. However, it's not going to be an accurate model of what everybody does, and therein lies some element of danger. > Then what could you have confidence in? Real-world experience. Which is hard to get if we don't ever commit any patches, but a good argument for (a) having them tested by multiple different hackers who invent test cases independently and (b) some configurability where we can reasonably include it, so that if anyone does experience problems they have an escape. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations 2022-02-04 19:45 Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> 2022-02-04 20:30 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Peter Geoghegan <[email protected]> 2022-02-04 21:18 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> @ 2022-02-04 22:26 ` Peter Geoghegan <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Peter Geoghegan @ 2022-02-04 22:26 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]> On Fri, Feb 4, 2022 at 4:18 PM Robert Haas <[email protected]> wrote: > On Fri, Feb 4, 2022 at 3:31 PM Peter Geoghegan <[email protected]> wrote: > > Application B will already block pruning by VACUUM operations against > > application A's table, and so effectively blocks recording of the > > resultant free space in the FSM in your scenario. And so application A > > and application B should be considered the same application already. > > That's just how VACUUM works. > > Sure ... but that also sucks. If we consider application A and > application B to be the same application, then we're basing our > decision about what to do on information that is inaccurate. I agree that it sucks, but I don't think that it's particularly relevant to the FSM prototype patch that I included with v7 of the patch series. A heap page cannot be considered "closed" (either in the specific sense from the patch, or in any informal sense) when it has recently dead tuples. At some point we should invent a fallback path for pruning, that migrates recently dead tuples to some other subsidiary structure, retaining only forwarding information in the heap page. But even that won't change what I just said about closed pages (it'll just make it easier to return and fix things up later on). > I don't see it that way. There are cases where avoiding writes is > better, and cases where trying to cram everything into the fewest > possible ages is better. With the right test case you can make either > strategy look superior. The cost of reads is effectively much lower than writes with modern SSDs, in TCO terms. Plus when a FSM strategy like the one from the patch does badly according to a naive measure such as total table size, that in itself doesn't mean that we do worse with reads. In fact, it's quite the opposite. The benchmark showed that v7 of the patch did very slightly worse on overall space utilization, but far, far better on reads. In fact, the benefits for reads were far in excess of any efficiency gains for writes/with WAL. The greatest bottleneck is almost always latency on modern hardware [1]. It follows that keeping logically related data grouped together is crucial. Far more important than potentially using very slightly more space. The story I wanted to tell with the FSM patch was about open and closed pages being the right long term direction. More generally, we should emphasize managing page-level costs, and deemphasize managing tuple-level costs, which are much less meaningful. > What I think your test case has going for it > is that it is similar to something that a lot of people, really a ton > of people, actually do with PostgreSQL. However, it's not going to be > an accurate model of what everybody does, and therein lies some > element of danger. No question -- agreed. > > Then what could you have confidence in? > > Real-world experience. Which is hard to get if we don't ever commit > any patches, but a good argument for (a) having them tested by > multiple different hackers who invent test cases independently and (b) > some configurability where we can reasonably include it, so that if > anyone does experience problems they have an escape. I agree. [1] https://dl.acm.org/doi/10.1145/1022594.1022596 -- Peter Geoghegan ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index ce47a158ae..ddcdbbaf7e 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2308,7 +2308,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 5c2a7b7422..9c7b8bf162 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -806,10 +806,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -840,15 +837,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2060,7 +2052,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2072,7 +2063,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --3o7pc6dfau5a5hry Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index ce47a158ae..ddcdbbaf7e 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2308,7 +2308,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 5c2a7b7422..9c7b8bf162 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -806,10 +806,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -840,15 +837,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2060,7 +2052,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2072,7 +2063,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 10c1c3b616b..a1ec50ab7a8 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 248ff90904b..04ad14f70b3 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -287,7 +287,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f1d0d4b78e3..e35bd36e710 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -787,10 +787,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -821,15 +818,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0010-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 10c1c3b616b..a1ec50ab7a8 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 94a5e2da17c..bbdaa591891 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f1d0d4b78e3..e35bd36e710 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -787,10 +787,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -821,15 +818,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --3o7pc6dfau5a5hry Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --3o7pc6dfau5a5hry Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 10c1c3b616b..a1ec50ab7a8 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 248ff90904b..04ad14f70b3 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -287,7 +287,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f1d0d4b78e3..e35bd36e710 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -787,10 +787,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -821,15 +818,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0010-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 10c1c3b616b..a1ec50ab7a8 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 94a5e2da17c..bbdaa591891 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f1d0d4b78e3..e35bd36e710 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -787,10 +787,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -821,15 +818,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v12 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 973734e9ffa..2cb5fb18675 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2335,7 +2335,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed41..282dcb97919 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 19e99c81092..ff3e8bcdd6f 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -802,10 +802,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -836,15 +833,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2050,7 +2042,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2062,7 +2053,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --6jpz2j246qmht4bt Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v12-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 849cac3947..cf4387f443 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b9ba4f9fb3..bcf1497f67 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -795,10 +795,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -829,15 +826,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 10c1c3b616b..a1ec50ab7a8 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 248ff90904b..04ad14f70b3 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -287,7 +287,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f1d0d4b78e3..e35bd36e710 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -787,10 +787,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -821,15 +818,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0010-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres @ 2024-02-12 23:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw) With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult as a means of communication between table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block(). Note that this parameter was unused by heap AM's implementation of table_scan_bitmap_next_tuple(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2... --- src/backend/access/heap/heapam_handler.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 2 +- src/include/access/tableam.h | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 10c1c3b616b..a1ec50ab7a8 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TBMIterateResult *tbmres, TupleTableSlot *slot) { HeapScanDesc hscan = (HeapScanDesc) scan; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 94a5e2da17c..bbdaa591891 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Attempt to fetch tuple from AM. */ - if (!table_scan_bitmap_next_tuple(scan, tbmres, slot)) + if (!table_scan_bitmap_next_tuple(scan, slot)) { /* nothing more to look at on this page */ node->tbmres = tbmres = NULL; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f1d0d4b78e3..e35bd36e710 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -787,10 +787,7 @@ typedef struct TableAmRoutine * * This will typically read and pin the target block, and do the necessary * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). For some - * AMs it will make more sense to do all the work referencing `tbmres` - * contents here, for others it might be better to defer more work to - * scan_bitmap_next_tuple. + * make sense to perform tuple visibility checks at this time). * * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples * on the page have to be returned, otherwise the tuples at offsets in @@ -821,15 +818,10 @@ typedef struct TableAmRoutine * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * For some AMs it will make more sense to do all the work referencing - * `tbmres` contents in scan_bitmap_next_block, for others it might be - * better to defer more work to this callback. - * * Optional callback, but either both scan_bitmap_next_block and * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot); /* @@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - tbmres, slot); } -- 2.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
end of thread, other threads:[~2024-02-12 23:13 UTC | newest] Thread overview: 23+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-11-28 22:30 [PATCH 3/6] Be less verbose on variable names Alvaro Herrera <[email protected]> 2022-02-04 19:45 Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> 2022-02-04 20:30 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Peter Geoghegan <[email protected]> 2022-02-04 21:18 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Robert Haas <[email protected]> 2022-02-04 22:26 ` Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations Peter Geoghegan <[email protected]> 2024-02-12 23:13 [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v12 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-02-12 23:13 [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres 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