8000 revert: Generalize relation analyze in table AM interface · postgres/postgres@6377e12 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6377e12

Browse files
akorotkovakorotkov
committed
revert: Generalize relation analyze in table AM interface
This commit reverts 27bc177 and dd1f6b0. Per review by Andres Freund. Discussion: https://postgr.es/m/20240415201057.khoyxbwwxfgzomeo%40awork3.anarazel.de
1 parent bea97cd commit 6377e12

File tree

8 files changed

+107
-168
lines changed
  • commands
  • include
  • tools/pgindent

    • 8 files changed

    +107
    -168
    lines changed

    ‎src/backend/access/heap/heapam_handler.c

    Lines changed: 4 additions & 25 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1002,7 +1002,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
    10021002
    * until heapam_scan_analyze_next_tuple() returns false. That is until all the
    10031003
    * items of the heap page are analyzed.
    10041004
    */
    1005-
    bool
    1005+
    static bool
    10061006
    heapam_scan_analyze_next_block(TableScanDesc scan, ReadStream *stream)
    10071007
    {
    10081008
    HeapScanDesc hscan = (HeapScanDesc) scan;
    @@ -1026,17 +1026,7 @@ heapam_scan_analyze_next_block(TableScanDesc scan, ReadStream *stream)
    10261026
    return true;
    10271027
    }
    10281028

    1029-
    /*
    1030-
    * Iterate over tuples in the block selected with
    1031-
    * heapam_scan_analyze_next_block(). If a tuple that's suitable for sampling
    1032-
    * is found, true is returned and a tuple is stored in `slot`. When no more
    1033-
    * tuples for sampling, 8000 false is returned and the pin and lock acquired by
    1034-
    * heapam_scan_analyze_next_block() are released.
    1035-
    *
    1036-
    * *liverows and *deadrows are incremented according to the encountered
    1037-
    * tuples.
    1038-
    */
    1039-
    bool
    1029+
    static bool
    10401030
    heapam_scan_analyze_next_tuple(TableScanDesc scan, TransactionId OldestXmin,
    10411031
    double *liverows, double *deadrows,
    10421032
    TupleTableSlot *slot)
    @@ -2593,18 +2583,6 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
    25932583
    }
    25942584
    }
    25952585

    2596-
    /*
    2597-
    * heapap_analyze -- implementation of relation_analyze() for heap
    2598-
    * table access method
    2599-
    */
    2600-
    static void
    2601-
    heapam_analyze(Relation relation, AcquireSampleRowsFunc *func,
    2602-
    BlockNumber *totalpages, BufferAccessStrategy bstrategy)
    2603-
    {
    2604-
    block_level_table_analyze(relation, func, totalpages, bstrategy,
    2605-
    heapam_scan_analyze_next_block,
    2606-
    heapam_scan_analyze_next_tuple);
    2607-
    }
    26082586

    26092587
    /* ------------------------------------------------------------------------
    26102588
    * Definition of the heap table access method.
    @@ -2652,9 +2630,10 @@ static const TableAmRoutine heapam_methods = {
    26522630
    .relation_copy_data = heapam_relation_copy_data,
    26532631
    .relation_copy_for_cluster = heapam_relation_copy_for_cluster,
    26542632
    .relation_vacuum = heap_vacuum_rel,
    2633+
    .scan_analyze_next_block = heapam_scan_analyze_next_block,
    2634+
    .scan_analyze_next_tuple = heapam_scan_analyze_next_tuple,
    26552635
    .index_build_range_scan = heapam_index_build_range_scan,
    26562636
    .index_validate_scan = heapam_index_validate_scan,
    2657-
    .relation_analyze = heapam_analyze,
    26582637

    26592638
    .relation_size = table_block_relation_size,
    26602639
    .relation_needs_toast_table = heapam_relation_needs_toast_table,

    ‎src/backend/access/table/tableamapi.c

    Lines changed: 2 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -81,6 +81,8 @@ GetTableAmRoutine(Oid amhandler)
    8181
    Assert(routine->relation_copy_data != NULL);
    8282
    Assert(routine->relation_copy_for_cluster != NULL);
    8383
    Assert(routine->relation_vacuum != NULL);
    84+
    Assert(routine->scan_analyze_next_block != NULL);
    85+
    Assert(routine->scan_analyze_next_tuple != NULL);
    8486
    Assert(routine->index_build_range_scan != NULL);
    8587
    Assert(routine->index_validate_scan != NULL);
    8688

    ‎src/backend/commands/analyze.c

    Lines changed: 16 additions & 39 deletions
    Original file line numberDiff line numberDiff line change
    @@ -17,7 +17,6 @@
    1717
    #include <math.h>
    1818

    1919
    #include "access/detoast.h"
    20-
    #include "access/heapam.h"
    2120
    #include "access/genam.h"
    2221
    #include "access/multixact.h"
    2322
    #include "access/relation.h"
    @@ -76,8 +75,6 @@ int default_statistics_target = 100;
    7675
    /* A few variables that don't seem worth passing around as parameters */
    7776
    static MemoryContext anl_context = NULL;
    7877
    static BufferAccessStrategy vac_strategy;
    79-
    static ScanAnalyzeNextBlockFunc scan_analyze_next_block;
    80-
    static ScanAnalyzeNextTupleFunc scan_analyze_next_tuple;
    8178

    8279

    8380
    static void do_analyze_rel(Relation onerel,
    @@ -90,6 +87,9 @@ static void compute_index_stats(Relation onerel, double totalrows,
    9087
    MemoryContext col_context);
    9188
    static VacAttrStats *examine_attribute(Relation onerel, int attnum,
    9289
    Node *index_expr);
    90+
    static int acquire_sample_rows(Relation onerel, int elevel,
    91+
    HeapTuple *rows, int targrows,
    92+
    double *totalrows, double *totaldeadrows);
    9393
    static int compare_rows(const void *a, const void *b, void *arg);
    9494
    static int acquire_inherited_sample_rows(Relation onerel, int elevel,
    9595
    HeapTuple *rows, int targrows,
    @@ -190,12 +190,10 @@ analyze_rel(Oid relid, RangeVar *relation,
    190190
    if (onerel->rd_rel->relkind == RELKIND_RELATION ||
    191191
    onerel->rd_rel->relkind == RELKIND_MATVIEW)
    192192
    {
    193-
    /*
    194-
    * Get row acquisition function, blocks and tuples iteration callbacks
    195-
    * provided by table AM
    196-
    */
    197-
    table_relation_analyze(onerel, &acquirefunc,
    198-
    &relpages, vac_strategy);
    193+
    /* Regular table, so we'll use the regular row acquisition function */
    194+
    acquirefunc = acquire_sample_rows;
    195+
    /* Also get regular table's size */
    196+
    relpages = RelationGetNumberOfBlocks(onerel);
    199197
    }
    200198
    else if (onerel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
    201199
    {
    @@ -1119,17 +1117,15 @@ block_sampling_read_stream_next(ReadStream *stream,
    11191117
    }
    11201118

    11211119
    /*
    1122-
    * acquire_sample_rows -- acquire a random sample of rows from the
    1123-
    * block-based relation
    1120+
    * acquire_sample_rows -- acquire a random sample of rows from the table
    11241121
    *
    11251122
    * Selected rows are returned in the caller-allocated array rows[], which
    11261123
    * must have at least targrows entries.
    11271124
    * The actual number of rows selected is returned as the function result.
    1128-
    * We also estimate the total numbers of live and dead rows in the relation,
    1125+
    * We also estimate the total numbers of live and dead rows in the table,
    11291126
    * and return them into *totalrows and *totaldeadrows, respectively.
    11301127
    *
    1131-
    * The returned list of tuples is in order by physical position in the
    1132-
    * relation.
    1128+
    * The returned list of tuples is in order by physical position in the table.
    11331129
    * (We will rely on this later to derive correlation estimates.)
    11341130
    *
    11351131
    * As of May 2004 we use a new two-stage method: Stage one selects up
    @@ -1151,7 +1147,7 @@ block_sampling_read_stream_next(ReadStream *stream,
    11511147
    * look at a statistically unbiased set of blocks, we should get
    11521148
    * unbiased estimates of the average numbers of live and dead rows per
    11531149
    * block. The previous sampling method put too much credence in the row
    1154-
    * density near the start of the relation.
    1150+
    * density near the start of the table.
    11551151
    */
    11561152
    static int
    11571153
    acquire_sample_rows(Relation onerel, int elevel,
    @@ -1204,11 +1200,11 @@ acquire_sample_rows(Relation onerel, int elevel,
    12041200
    0);
    12051201

    12061202
    /* Outer loop over blocks to sample */
    1207-
    while (scan_analyze_next_block(scan, stream))
    1203+
    while (table_scan_analyze_next_block(scan, stream))
    12081204
    {
    12091205
    vacuum_delay_point();
    12101206

    1211-
    while (scan_analyze_next_tuple(scan, OldestXmin, &liverows, &deadrows, slot))
    1207+
    while (table_scan_analyze_next_tuple(scan, OldestXmin, &liverows, &deadrows, slot))
    12121208
    {
    12131209
    /*
    12141210
    * The first targrows sample rows are simply copied into the
    @@ -1331,25 +1327,6 @@ compare_rows(const void *a, const void *b, void *arg)
    13311327
    return 0;
    13321328
    }
    13331329

    1334-
    /*
    1335-
    * block_level_table_analyze -- implementation of relation_analyze() for
    1336-
    * block-level table access methods
    1337-
    */
    1338-
    void
    1339-
    block_level_table_analyze(Relation relation,
    1340-
    AcquireSampleRowsFunc *func,
    1341-
    BlockNumber *totalpages,
    1342-
    BufferAccessStrategy bstrategy,
    1343-
    ScanAnalyzeNextBlockFunc scan_analyze_next_block_cb,
    1344-
    ScanAnalyzeNextTupleFunc scan_analyze_next_tuple_cb)
    1345-
    {
    1346-
    *func = acquire_sample_rows;
    1347-
    *totalpages = RelationGetNumberOfBlocks(relation);
    1348-
    vac_strategy = bstrategy;
    1349-
    scan_analyze_next_block = scan_analyze_next_block_cb;
    1350-
    scan_analyze_next_tuple = scan_analyze_next_tuple_cb;
    1351-
    }
    1352-
    13531330

    13541331
    /*
    13551332
    * acquire_inherited_sample_rows -- acquire sample rows from inheritance tree
    @@ -1439,9 +1416,9 @@ acquire_inherited_sample_rows(Relation onerel, int elevel,
    14391416
    if (childrel->rd_rel->relkind == RELKIND_RELATION ||
    14401417
    childrel->rd_rel->relkind == RELKIND_MATVIEW)
    14411418
    {
    1442-
    /* Use row acquisition function provided by table AM */
    1443-
    table_relation_analyze(childrel, &acquirefunc,
    1444-
    &relpages, vac_strategy);
    1419+
    /* Regular table, so use the regular row acquisition function */
    1420+
    acquirefunc = acquire_sample_rows;
    1421+
    relpages = RelationGetNumberOfBlocks(childrel);
    14451422
    }
    14461423
    else if (childrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
    14471424
    {

    ‎src/include/access/heapam.h

    Lines changed: 0 additions & 8 deletions
    Original file line numberDiff line numberDiff line change
    @@ -409,14 +409,6 @@ extern bool HeapTupleHeaderIsOnlyLocked(HeapTupleHeader tuple);
    409409
    extern bool HeapTupleIsSurelyDead(HeapTuple htup,
    410410
    struct GlobalVisState *vistest);
    411411

    412-
    /* in heap/heapam_handler.c*/
    413-
    extern bool heapam_scan_analyze_next_block(TableScanDesc scan,
    414-
    ReadStream *stream);
    415-
    extern bool heapam_scan_analyze_next_tuple(TableScanDesc scan,
    416-
    TransactionId OldestXmin,
    417-
    double *liverows, double *deadrows,
    418-
    TupleTableSlot *slot);
    419-
    420412
    /*
    421413
    * To avoid leaking too much knowledge about reorderbuffer implementation
    422414
    * details this is implemented in reorderbuffer.c not heapam_visibility.c

    ‎src/include/access/tableam.h

    Lines changed: 79 additions & 22 deletions
    Original file line numberDiff line numberDiff line change
    @@ -20,8 +20,8 @@
    2020
    #include "access/relscan.h"
    2121
    #include "access/sdir.h"
    2222
    #include "access/xact.h"
    23-
    #include "commands/vacuum.h"
    2423
    #include "executor/tuptable.h"
    24+
    #include "storage/read_stream.h"
    2525
    #include "utils/rel.h"
    2626
    #include "utils/snapshot.h"
    2727

    @@ -655,6 +655,50 @@ typedef struct TableAmRoutine
    655655
    struct VacuumParams *params,
    656656
    BufferAccessStrategy bstrategy);
    657657

    658+
    /*
    659+
    * Prepare to analyze the next block in the read stream. Returns false if
    660+
    * the stream is exhausted and true otherwise. The scan must have been
    661+
    * started with SO_TYPE_ANALYZE option.
    662+
    *
    663+
    * This routine holds a buffer pin and lock on the heap page. They are
    664+
    * held until heapam_scan_analyze_next_tuple() returns false. That is
    665+
    * until all the items of the heap page are analyzed.
    666+
    */
    667+
    668+
    /*
    669+
    * Prepare to analyze block `blockno` of `scan`. The scan has been started
    670+
    * with table_beginscan_analyze(). See also
    671+
    * table_scan_analyze_next_block().
    672+
    *
    673+
    * The callback may acquire resources like locks that are held until
    674+
    * table_scan_analyze_next_tuple() returns false. It e.g. can make sense
    675+
    * to hold a lock until all tuples on a block have been analyzed by
    676+
    * scan_analyze_next_tuple.
    677+
    *
    678+
    * The callback can return false if the block is not suitable for
    679+
    * sampling, e.g. because it's a metapage that could never contain tuples.
    680+
    *
    681+
    * XXX: This obviously is primarily suited for block-based AMs. It's not
    682+
    * clear what a good interface for non block based AMs would be, so there
    683+
    * isn't one yet.
    684+
    */
    685+
    bool (*scan_analyze_next_block) (TableScanDesc scan,
    686+
    ReadStream *stream);
    687+
    688+
    /*
    689+
    * See table_scan_analyze_next_tuple().
    690+
    *
    691+
    * Not every AM might have a meaningful concept of dead rows, in which
    692+
    * case it's OK to not increment *deadrows - but note that that may
    693+
    * influence autovacuum scheduling (see comment for relation_vacuum
    694+
    * callback).
    695+
    */
    696+
    bool (*scan_analyze_next_tuple) (TableScanDesc scan,
    697+
    TransactionId OldestXmin,
    698+
    double *liverows,
    699+
    double *deadrows,
    700+
    TupleTableSlot *slot);
    701+
    658702
    /* see table_index_build_range_scan for reference about parameters */
    659703
    double (*index_build_range_scan) (Relation table_rel,
    660704
    Relation index_rel,
    @@ -675,12 +719,6 @@ typedef struct TableAmRoutine
    675719
    Snapshot snapshot,
    676720
    struct ValidateIndexState *state);
    677721

    678-
    /* See table_relation_analyze() */
    679-
    void (*relation_analyze) (Relation relation,
    680-
    AcquireSampleRowsFunc *func,
    681-
    BlockNumber *totalpages,
    682-
    BufferAccessStrategy bstrategy);
    683-
    684722

    685723
    /* ------------------------------------------------------------------------
    686724
    * Miscellaneous functions.
    @@ -1682,6 +1720,40 @@ table_relation_vacuum(Relation rel, struct VacuumParams *params,
    16821720
    rel->rd_tableam->relation_vacuum(rel, params, bstrategy);
    16831721
    }
    16841722

    1723+
    /*
    1724+
    * Prepare to analyze the next block in the read stream. The scan needs to
    1725+
    * have been started with table_beginscan_analyze(). Note that this routine
    1726+
    * might acquire resources like locks that are held until
    1727+
    * table_scan_analyze_next_tuple() returns false.
    1728+
    *
    1729+
    * Returns false if block is unsuitable for sampling, true otherwise.
    1730+
    */
    1731+
    static inline bool
    1732+
    table_scan_analyze_next_block(TableScanDesc scan, ReadStream *stream)
    1733+
    {
    1734+
    return scan->rs_rd->rd_tableam->scan_analyze_next_block(scan, stream);
    1735+
    }
    1736+
    1737+
    /*
    1738+
    * Iterate over tuples in the block selected with
    1739+
    * table_scan_analyze_next_block() (which needs to have returned true, and
    1740+
    * this routine may not have returned false for the same block before). If a
    1741+
    * tuple that's suitable for sampling is found, true is returned and a tuple
    1742+
    * is stored in `slot`.
    1743+
    *
    1744+
    * *liverows and *deadrows are incremented according to the encountered
    1745+
    * tuples.
    1746+
    */
    1747+
    static inline bool
    1748+
    table_scan_analyze_next_tuple(TableScanDesc scan, TransactionId OldestXmin,
    1749+
    double *liverows, double *deadrows,
    1750+
    TupleTableSlot *slot)
    1751+
    {
    1752+
    return scan->rs_rd->rd_tableam->scan_analyze_next_tuple(scan, OldestXmin,
    1753+
    liverows, deadrows,
    1754+
    slot);
    1755+
    }
    1756+
    16851757
    /*
    16861758
    * table_index_build_scan - scan the table to find tuples to be indexed
    16871759
    *
    @@ -1787,21 +1859,6 @@ table_index_validate_scan(Relation table_rel,
    17871859
    state);
    17881860
    }
    17891861

    1790-
    /*
    1791-
    * table_relation_analyze - fill the infromation for a sampling statistics
    1792-
    * acquisition
    1793-
    *
    1794-
    * The pointer to a function that will collect sample rows from the table
    1795-
    * should be stored to `*func`, plus the estimated size of the table in pages
    1796-
    * should br stored to `*totalpages`.
    1797-
    */
    1798-
    static inline void
    1799-
    table_relation_analyze(Relation relation, AcquireSampleRowsFunc *func,
    1800-
    BlockNumber *totalpages, BufferAccessStrategy bstrategy)
    1801-
    {
    1802-
    relation->rd_tableam->relation_analyze(relation, func,
    1803-
    totalpages, bstrategy);
    1804-
    }
    18051862

    18061863
    /* ----------------------------------------------------------------------------
    18071864
    * Miscellaneous functionality

    0 commit comments

    Comments
     (0)
    0