From 5886698acb05231d0150e779011796c594cb7ed2 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Mon, 8 Apr 2024 12:30:45 +0300 Subject: [PATCH v3] Generalize acquire_sample_rows() Reported-by: Bug: Discussion: Author: Reviewed-by: Tested-by: Backpatch-through: --- contrib/file_fdw/file_fdw.c | 13 ++-- contrib/postgres_fdw/postgres_fdw.c | 13 ++-- src/backend/commands/analyze.c | 96 +++++++++++++++++++---------- src/include/access/tableam.h | 21 ++++++- src/include/commands/vacuum.h | 59 +++++++++++++++++- src/include/foreign/fdwapi.h | 3 +- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 159 insertions(+), 47 deletions(-) diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 249d82d3a0..9c162c099f 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -139,7 +139,8 @@ static void fileReScanForeignScan(ForeignScanState *node); static void fileEndForeignScan(ForeignScanState *node); static bool fileAnalyzeForeignTable(Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages); + BlockNumber *totalpages, + void **arg); static bool fileIsForeignScanParallelSafe(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte); @@ -162,7 +163,8 @@ static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel, Cost *startup_cost, Cost *total_cost); static int file_acquire_sample_rows(Relation onerel, int elevel, HeapTuple *rows, int targrows, - double *totalrows, double *totaldeadrows); + double *totalrows, double *totaldeadrows, + void *arg); /* @@ -806,7 +808,8 @@ fileEndForeignScan(ForeignScanState *node) static bool fileAnalyzeForeignTable(Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages) + BlockNumber *totalpages, + void **arg) { char *filename; bool is_program; @@ -845,6 +848,7 @@ fileAnalyzeForeignTable(Relation relation, *totalpages = 1; *func = file_acquire_sample_rows; + *arg = NULL; return true; } @@ -1122,7 +1126,8 @@ estimate_costs(PlannerInfo *root, RelOptInfo *baserel, static int file_acquire_sample_rows(Relation onerel, int elevel, HeapTuple *rows, int targrows, - double *totalrows, double *totaldeadrows) + double *totalrows, double *totaldeadrows, + void *arg) { int numrows = 0; double rowstoskip = -1; /* -1 means not set yet */ diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 142dcfc995..08e53aa99d 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -400,7 +400,8 @@ static void postgresExecForeignTruncate(List *rels, bool restart_seqs); static bool postgresAnalyzeForeignTable(Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages); + BlockNumber *totalpages, + void **arg); static List *postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid); static void postgresGetForeignJoinPaths(PlannerInfo *root, @@ -501,7 +502,8 @@ static void process_query_params(ExprContext *econtext, static int postgresAcquireSampleRowsFunc(Relation relation, int elevel, HeapTuple *rows, int targrows, double *totalrows, - double *totaldeadrows); + double *totaldeadrows, + void *arg); static void analyze_row_processor(PGresult *res, int row, PgFdwAnalyzeState *astate); static void produce_tuple_asynchronously(AsyncRequest *areq, bool fetch); @@ -4921,7 +4923,8 @@ process_query_params(ExprContext *econtext, static bool postgresAnalyzeForeignTable(Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages) + BlockNumber *totalpages, + void **arg) { ForeignTable *table; UserMapping *user; @@ -4931,6 +4934,7 @@ postgresAnalyzeForeignTable(Relation relation, /* Return the row-analysis function pointer */ *func = postgresAcquireSampleRowsFunc; + *arg = NULL; /* * Now we have to get the number of pages. It's annoying that the ANALYZE @@ -5057,7 +5061,8 @@ static int postgresAcquireSampleRowsFunc(Relation relation, int elevel, HeapTuple *rows, int targrows, double *totalrows, - double *totaldeadrows) + double *totaldeadrows, + void *arg) { PgFdwAnalyzeState astate; ForeignTable *table; diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index da27a13a3f..b2c4b99357 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -80,7 +80,8 @@ static BufferAccessStrategy vac_strategy; static void do_analyze_rel(Relation onerel, VacuumParams *params, List *va_cols, - AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, + AcquireSampleRowsFunc acquirefunc, + void *acquirefuncArg, BlockNumber relpages, bool inh, bool in_outer_xact, int elevel); static void compute_index_stats(Relation onerel, double totalrows, AnlIndexData *indexdata, int nindexes, @@ -88,9 +89,6 @@ static void compute_index_stats(Relation onerel, double totalrows, MemoryContext col_context); static VacAttrStats *examine_attribute(Relation onerel, int attnum, Node *index_expr); -static int acquire_sample_rows(Relation onerel, int elevel, - HeapTuple *rows, int targrows, - double *totalrows, double *totaldeadrows); static int compare_rows(const void *a, const void *b, void *arg); static int acquire_inherited_sample_rows(Relation onerel, int elevel, HeapTuple *rows, int targrows, @@ -116,6 +114,7 @@ analyze_rel(Oid relid, RangeVar *relation, Relation onerel; int elevel; AcquireSampleRowsFunc acquirefunc = NULL; + void *acquirefuncArg = NULL; BlockNumber relpages = 0; /* Select logging level */ @@ -191,9 +190,12 @@ analyze_rel(Oid relid, RangeVar *relation, if (onerel->rd_rel->relkind == RELKIND_RELATION || onerel->rd_rel->relkind == RELKIND_MATVIEW) { - /* Use row acquisition function provided by table AM */ + /* + * Get row acquisition function, blocks and tuples iteration + * callbacks provided by table AM + */ table_relation_analyze(onerel, &acquirefunc, - &relpages, vac_strategy); + &relpages, vac_strategy, &acquirefuncArg); } else if (onerel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) { @@ -209,7 +211,8 @@ analyze_rel(Oid relid, RangeVar *relation, if (fdwroutine->AnalyzeForeignTable != NULL) ok = fdwroutine->AnalyzeForeignTable(onerel, &acquirefunc, - &relpages); + &relpages, + &acquirefuncArg); if (!ok) { @@ -248,15 +251,15 @@ analyze_rel(Oid relid, RangeVar *relation, * tables, which don't contain any rows. */ if (onerel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) - do_analyze_rel(onerel, params, va_cols, acquirefunc, + do_analyze_rel(onerel, params, va_cols, acquirefunc, acquirefuncArg, relpages, false, in_outer_xact, elevel); /* * If there are child tables, do recursive ANALYZE. */ if (onerel->rd_rel->relhassubclass) - do_analyze_rel(onerel, params, va_cols, acquirefunc, relpages, - true, in_outer_xact, elevel); + do_analyze_rel(onerel, params, va_cols, acquirefunc, acquirefuncArg, + relpages, true, in_outer_xact, elevel); /* * Close source relation now, but keep lock so that no one deletes it @@ -272,15 +275,16 @@ analyze_rel(Oid relid, RangeVar *relation, /* * do_analyze_rel() -- analyze one relation, recursively or not * - * Note that "acquirefunc" is only relevant for the non-inherited case. - * For the inherited case, acquire_inherited_sample_rows() determines the - * appropriate acquirefunc for each child table. + * Note that "acquirefunc" and “acquirefuncArg” are only relevant for the + * non-inherited case. For the inherited case, acquire_inherited_sample_rows() + * determines the appropriate acquirefunc and acquirefuncArg for each child + * table. */ static void do_analyze_rel(Relation onerel, VacuumParams *params, List *va_cols, AcquireSampleRowsFunc acquirefunc, - BlockNumber relpages, bool inh, bool in_outer_xact, - int elevel) + void *acquirefuncArg, BlockNumber relpages, bool inh, + bool in_outer_xact, int elevel) { int attr_cnt, tcnt, @@ -526,7 +530,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params, else numrows = (*acquirefunc) (onerel, elevel, rows, targrows, - &totalrows, &totaldeadrows); + &totalrows, &totaldeadrows, + acquirefuncArg); /* * Compute the statistics. Temporary results during the calculations for @@ -1117,15 +1122,17 @@ block_sampling_read_stream_next(ReadStream *stream, } /* - * acquire_sample_rows -- acquire a random sample of rows from the heap + * acquire_sample_rows -- acquire a random sample of rows from the + * block-based relation * * Selected rows are returned in the caller-allocated array rows[], which * must have at least targrows entries. * The actual number of rows selected is returned as the function result. - * We also estimate the total numbers of live and dead rows in the heap, + * We also estimate the total numbers of live and dead rows in the relation, * and return them into *totalrows and *totaldeadrows, respectively. * - * The returned list of tuples is in order by physical position in the heap. + * The returned list of tuples is in order by physical position in the + * relation. * (We will rely on this later to derive correlation estimates.) * * As of May 2004 we use a new two-stage method: Stage one selects up @@ -1147,13 +1154,15 @@ block_sampling_read_stream_next(ReadStream *stream, * look at a statistically unbiased set of blocks, we should get * unbiased estimates of the average numbers of live and dead rows per * block. The previous sampling method put too much credence in the row - * density near the start of the heap. + * density near the start of the relation. */ -static int +int acquire_sample_rows(Relation onerel, int elevel, HeapTuple *rows, int targrows, - double *totalrows, double *totaldeadrows) + double *totalrows, double *totaldeadrows, + void *arg) { + AcquireSampleRowsArg *cb = (AcquireSampleRowsArg *) arg; int numrows = 0; /* # rows now in reservoir */ double samplerows = 0; /* total # rows collected */ double liverows = 0; /* # live rows seen */ @@ -1188,7 +1197,7 @@ acquire_sample_rows(Relation onerel, int elevel, /* Prepare for sampling rows */ reservoir_init_selection_state(&rstate, targrows); - scan = heap_beginscan(onerel, NULL, 0, NULL, NULL, SO_TYPE_ANALYZE); + scan = table_beginscan_analyze(onerel); slot = table_slot_create(onerel, NULL); stream = read_stream_begin_relation(READ_STREAM_MAINTENANCE, @@ -1200,11 +1209,11 @@ acquire_sample_rows(Relation onerel, int elevel, 0); /* Outer loop over blocks to sample */ - while (heapam_scan_analyze_next_block(scan, stream)) + while (cb->scan_analyze_next_block(scan, stream)) { vacuum_delay_point(); - while (heapam_scan_analyze_next_tuple(scan, OldestXmin, &liverows, &deadrows, slot)) + while (cb->scan_analyze_next_tuple(scan, OldestXmin, &liverows, &deadrows, slot)) { /* * The first targrows sample rows are simply copied into the @@ -1256,7 +1265,7 @@ acquire_sample_rows(Relation onerel, int elevel, read_stream_end(stream); ExecDropSingleTupleTableSlot(slot); - heap_endscan(scan); + table_endscan(scan); /* * If we didn't find as many tuples as we wanted then we're done. No sort @@ -1333,10 +1342,18 @@ compare_rows(const void *a, const void *b, void *arg) */ void heapam_analyze(Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages, BufferAccessStrategy bstrategy) + BlockNumber *totalpages, BufferAccessStrategy bstrategy, + void **arg) { + static AcquireSampleRowsArg cb = + { + .scan_analyze_next_block = heapam_scan_analyze_next_block, + .scan_analyze_next_tuple = heapam_scan_analyze_next_tuple + }; + *func = acquire_sample_rows; *totalpages = RelationGetNumberOfBlocks(relation); + *arg = &cb; vac_strategy = bstrategy; } @@ -1349,7 +1366,7 @@ heapam_analyze(Relation relation, AcquireSampleRowsFunc *func, * We fail and return zero if there are no inheritance children, or if all * children are foreign tables that don't support ANALYZE. */ -static int +int acquire_inherited_sample_rows(Relation onerel, int elevel, HeapTuple *rows, int targrows, double *totalrows, double *totaldeadrows) @@ -1357,6 +1374,7 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, List *tableOIDs; Relation *rels; AcquireSampleRowsFunc *acquirefuncs; + void **acquirefuncArgs; double *relblocks; double totalblocks; int numrows, @@ -1396,12 +1414,15 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, } /* - * Identify acquirefuncs to use, and count blocks in all the relations. + * Identify acquirefunc-s and acquirefuncArg-s to use, and count blocks + * in all the relations. * The result could overflow BlockNumber, so we use double arithmetic. */ rels = (Relation *) palloc(list_length(tableOIDs) * sizeof(Relation)); acquirefuncs = (AcquireSampleRowsFunc *) palloc(list_length(tableOIDs) * sizeof(AcquireSampleRowsFunc)); + acquirefuncArgs = (void **) + palloc(list_length(tableOIDs) * sizeof(void *)); relblocks = (double *) palloc(list_length(tableOIDs) * sizeof(double)); totalblocks = 0; nrels = 0; @@ -1411,6 +1432,7 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, Oid childOID = lfirst_oid(lc); Relation childrel; AcquireSampleRowsFunc acquirefunc = NULL; + void *acquirefuncArg = NULL; BlockNumber relpages = 0; /* We already got the needed lock */ @@ -1429,9 +1451,13 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, if (childrel->rd_rel->relkind == RELKIND_RELATION || childrel->rd_rel->relkind == RELKIND_MATVIEW) { - /* Use row acquisition function provided by table AM */ + /* + * Get row acquisition function, blocks and tuples iteration + * callbacks provided by table AM + */ table_relation_analyze(childrel, &acquirefunc, - &relpages, vac_strategy); + &relpages, vac_strategy, + &acquirefuncArg); } else if (childrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) { @@ -1447,7 +1473,8 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, if (fdwroutine->AnalyzeForeignTable != NULL) ok = fdwroutine->AnalyzeForeignTable(childrel, &acquirefunc, - &relpages); + &relpages, + &acquirefuncArg); if (!ok) { @@ -1475,6 +1502,7 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, has_child = true; rels[nrels] = childrel; acquirefuncs[nrels] = acquirefunc; + acquirefuncArgs[nrels] = acquirefuncArg; relblocks[nrels] = (double) relpages; totalblocks += (double) relpages; nrels++; @@ -1506,6 +1534,7 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, { Relation childrel = rels[i]; AcquireSampleRowsFunc acquirefunc = acquirefuncs[i]; + void *acquirefuncArg = acquirefuncArgs[i]; double childblocks = relblocks[i]; /* @@ -1544,7 +1573,8 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, /* Fetch a random sample of the child's rows */ childrows = (*acquirefunc) (childrel, elevel, rows + numrows, childtargrows, - &trows, &tdrows); + &trows, &tdrows, + acquirefuncArg); /* We may need to convert from child's rowtype to parent's */ if (childrows > 0 && diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index ec827ac12b..f28044d834 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -692,7 +692,8 @@ typedef struct TableAmRoutine void (*relation_analyze) (Relation relation, AcquireSampleRowsFunc *func, BlockNumber *totalpages, - BufferAccessStrategy bstrategy); + BufferAccessStrategy bstrategy, + void **arg); /* ------------------------------------------------------------------------ @@ -1020,6 +1021,19 @@ table_beginscan_tid(Relation rel, Snapshot snapshot) return rel->rd_tableam->scan_begin(rel, snapshot, 0, NULL, NULL, flags); } +/* + * table_beginscan_analyze is an alternative entry point for setting up a + * TableScanDesc for an ANALYZE scan. As with bitmap scans, it's worth using + * the same data structure although the behavior is rather different. + */ +static inline TableScanDesc +table_beginscan_analyze(Relation rel) +{ + uint32 flags = SO_TYPE_ANALYZE; + + return rel->rd_tableam->scan_begin(rel, NULL, 0, NULL, NULL, flags); +} + /* * End relation scan. */ @@ -1868,10 +1882,11 @@ table_index_validate_scan(Relation table_rel, */ static inline void table_relation_analyze(Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages, BufferAccessStrategy bstrategy) + BlockNumber *totalpages, BufferAccessStrategy bstrategy, + void **arg) { relation->rd_tableam->relation_analyze(relation, func, - totalpages, bstrategy); + totalpages, bstrategy, arg); } /* ---------------------------------------------------------------------------- diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 9514f8b2fd..5d06c0a567 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -21,9 +21,11 @@ #include "catalog/pg_class.h" #include "catalog/pg_statistic.h" #include "catalog/pg_type.h" +#include "executor/tuptable.h" #include "parser/parse_node.h" #include "storage/buf.h" #include "storage/lock.h" +#include "storage/read_stream.h" #include "utils/relcache.h" /* @@ -189,7 +191,8 @@ typedef struct VacAttrStats typedef int (*AcquireSampleRowsFunc) (Relation relation, int elevel, HeapTuple *rows, int targrows, double *totalrows, - double *totaldeadrows); + double *totaldeadrows, + void *arg); /* flag bits for VacuumParams->options */ #define VACOPT_VACUUM 0x01 /* do VACUUM */ @@ -390,12 +393,64 @@ extern void parallel_vacuum_cleanup_all_indexes(ParallelVacuumState *pvs, extern void parallel_vacuum_main(dsm_segment *seg, shm_toc *toc); /* in commands/analyze.c */ + +struct TableScanDescData; + +/* The struct to be passed as '*arg' to acquire_sample_rows */ +typedef struct +{ + /* + * Prepare to analyze block from `stream` of `scan`. The scan has been + * started with table_beginscan_analyze(). + * + * The callback may acquire resources like locks that are held until + * (*scan_analyze_next_tuple)() returns false. In some cases it could be + * useful to hold a lock until all tuples in a block have been analyzed + * by (*scan_analyze_next_tuple)(). + * + * The callback can return false if the block is not suitable for + * sampling, e.g. because it's a metapage that could never contain tuples. + * + * This is primarily suited for block-based AMs. It's not clear what a + * good interface for non block-based AMs would be, so there isn't one + * yet and sampling using a custom implementation of acquire_sample_rows + * may be preferred. + */ + bool (*scan_analyze_next_block) (struct TableScanDescData *scan, + ReadStream *stream); + + /* + * Iterate over tuples in the block selected with + * (*scan_analyze_next_block)() (which needs to have returned true, and + * this routine may not have returned false for the same block before). If + * a tuple that's suitable for sampling is found, true is returned and a + * tuple is stored in `slot`. + * + * *liverows and *deadrows are incremented according to the encountered + * tuples. + * + * Not every AM might have a meaningful concept of dead rows, in which + * case it's OK to not increment *deadrows - but note that that may + * influence autovacuum scheduling (see comment for relation_vacuum + * callback). + */ + bool (*scan_analyze_next_tuple) (struct TableScanDescData *scan, + TransactionId OldestXmin, + double *liverows, + double *deadrows, + TupleTableSlot *slot); +} AcquireSampleRowsArg; + extern void analyze_rel(Oid relid, RangeVar *relation, VacuumParams *params, List *va_cols, bool in_outer_xact, BufferAccessStrategy bstrategy); +extern int acquire_sample_rows(Relation onerel, int elevel, + HeapTuple *rows, int targrows, + double *totalrows, double *totaldeadrows, + void *arg); extern void heapam_analyze(Relation relation, AcquireSampleRowsFunc *func, BlockNumber *totalpages, - BufferAccessStrategy bstrategy); + BufferAccessStrategy bstrategy, void **arg); extern bool std_typanalyze(VacAttrStats *stats); diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h index 0968e0a01e..ebea559e53 100644 --- a/src/include/foreign/fdwapi.h +++ b/src/include/foreign/fdwapi.h @@ -151,7 +151,8 @@ typedef void (*ExplainDirectModify_function) (ForeignScanState *node, typedef bool (*AnalyzeForeignTable_function) (Relation relation, AcquireSampleRowsFunc *func, - BlockNumber *totalpages); + BlockNumber *totalpages, + void **arg); typedef List *(*ImportForeignSchema_function) (ImportForeignSchemaStmt *stmt, Oid serverOid); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index cb78f11119..0156ef27ab 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -22,6 +22,7 @@ AclItem AclMaskHow AclMode AclResult +AcquireSampleRowsArg AcquireSampleRowsFunc ActionList ActiveSnapshotElt -- 2.39.2 (Apple Git-143)