public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 3/6] Be less verbose on variable names 20+ messages / 3 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread
* Re: protocol-level wait-for-LSN @ 2024-10-31 13:25 Jelte Fennema-Nio <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Jelte Fennema-Nio @ 2024-10-31 13:25 UTC (permalink / raw) To: Jesper Pedersen <[email protected]>; +Cc: Ants Aasma <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers On Thu, 31 Oct 2024 at 13:59, Jesper Pedersen <[email protected]> wrote: > And, it will be confusing to clean-room implementations: When you have > this startup parameter then you get these message formats, when you have > this startup parameter then you get these message formats -- and what > about combinations ? Like Tatsuo-san stated up-thread. I really don't understand why you think that's so difficult. To be clear, no client is forced to implement any of these protocol options. And all of these protocol options would be documented in the official protocol docs. For instance the ReadyForQuery docs on the "Message Formats" page in the docs could easily be made to look like the following, which imho would be very clear to any implementer of the protocol about ordering of these fields: ReadyForQuery (B) Byte1('Z') Identifies the message type. ReadyForQuery is sent whenever the backend is ready for a new query cycle. Int32 Length of message contents in bytes, including self. Int64: Only present if protocol option wait_for_lsn is set to 1 by the client The LSN at time of commit Int64: Only present if protocol option query_time is set to 1 by the client Time it took to run the query in microseconds Byte1 Current backend transaction status indicator. Possible values are 'I' if idle (not in a transaction block); 'T' if in a transaction block; or 'E' if in a failed transaction block (queries will be rejected until block is ended). > You are also assuming that all PostgreSQL protocol implementations uses > the Length (Int32) field very strict - so when one developer adds the > startup parameter, but doesn't change the underlying implementation > everything will break. Yes... But that seems equivalent to saying: If a developer of a Postgres client advertises that they support protocol v4, but don't actually implement it, then everything will break. i.e. it's the job of the client author to not send protocol options that it doesn't know anything about. Just like it's the job of the client author not to request versions that it does not know anything about. > The protocol format must be 100% clear and well-documented in all cases. Agreed. See above. > If this door is open then it has to very clear how multiple startup > parameters are handled at the protocol level, and that is a much bigger > fish because what happens if extensions add startup parameters as well. Postgres extensions **cannot** add such startup parameters. Heikki already mentioned that the naming was confusing in the docs. At this point in time we're only discussing protocol changes that are coming from Postgres core (which is already a contentious enough topic). ^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2024-10-31 13:25 UTC | newest] Thread overview: 20+ 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]> 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 v8 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 v7 08/13] 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 v7 08/13] 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 v12 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 v10 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]> 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 v10 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 v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]> 2024-10-31 13:25 Re: protocol-level wait-for-LSN Jelte Fennema-Nio <[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