8000 Refactor option handling of CLUSTER, REINDEX and VACUUM · postgrespro/postgres@a3dc926 · GitHub
[go: up one dir, main page]
Skip to content

    • Commit a3dc926

    Browse files
    michaelpqmichaelpq
    committed
    Refactor option handling of CLUSTER, REINDEX and VACUUM
    This continues the work done in b5913f6. All the options of those commands are changed to use hex values rather than enums to reduce the risk of compatibility bugs when introducing new options. Each option set is moved into a new structure that can be extended with more non-boolean options (this was already the case of VACUUM). The code of REINDEX is restructured so as manual REINDEX commands go through a single routine from utility.c, like VACUUM, to ease the allocation handling of option parameters when a command needs to go through multiple transactions. This can be used as a base infrastructure for future patches related to those commands, including reindex filtering and tablespace support. Per discussion with people mentioned below, as well as Alvaro Herrera and Peter Eisentraut. Author: Michael Paquier, Justin Pryzby Reviewed-by: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/X8riynBLwxAD9uKk@paquier.xyz
    1 parent 04eb75e commit a3dc926

    File tree

    11 files changed

    +175
    -149
    lines changed

    11 files changed

    +175
    -149
    lines changed

    ‎src/backend/catalog/index.c

    Lines changed: 12 additions & 10 deletions
    Original file line numberDiff line numberDiff line change
    @@ -3594,15 +3594,15 @@ IndexGetRelation(Oid indexId, bool missing_ok)
    35943594
    */
    35953595
    void
    35963596
    reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
    3597-
    int options)
    3597+
    ReindexParams *params)
    35983598
    {
    35993599
    Relation iRel,
    36003600
    heapRelation;
    36013601
    Oid heapId;
    36023602
    IndexInfo *indexInfo;
    36033603
    volatile bool skipped_constraint = false;
    36043604
    PGRUsage ru0;
    3605-
    bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
    3605+
    bool progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
    36063606

    36073607
    pg_rusage_init(&ru0);
    36083608

    @@ -3611,12 +3611,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
    36113611
    * we only need to be sure no schema or data changes are going on.
    36123612
    */
    36133613
    heapId = IndexGetRelation(indexId,
    3614-
    (options & REINDEXOPT_MISSING_OK) != 0);
    3614+
    (params->options & REINDEXOPT_MISSING_OK) != 0);
    36153615
    /* if relation is missing, leave */
    36163616
    if (!OidIsValid(heapId))
    36173617
    return;
    36183618

    3619-
    if ((options & REINDEXOPT_MISSING_OK) != 0)
    3619+
    if ((params->options & REINDEXOPT_MISSING_OK) != 0)
    36203620
    heapRelation = try_table_open(heapId, ShareLock);
    36213621
    else
    36223622
    heapRelation = table_open(heapId, ShareLock);
    @@ -3792,7 +3792,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
    37923792
    }
    37933793

    37943794
    /* Log what we did */
    3795-
    if (options & REINDEXOPT_VERBOSE)
    3795+
    if ((params->options & REINDEXOPT_VERBOSE) != 0)
    37963796
    ereport(INFO,
    37973797
    (errmsg("index \"%s\" was reindexed",
    37983798
    get_rel_name(indexId)),
    @@ -3846,7 +3846,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
    38463846
    * index rebuild.
    38473847
    */
    38483848
    bool
    3849-
    reindex_relation(Oid relid, int flags, int options)
    3849+
    reindex_relation(Oid relid, int flags, ReindexParams *params)
    38503850
    {
    38513851
    Relation rel;
    38523852
    Oid toast_relid;
    @@ -3861,7 +3861,7 @@ reindex_relation(Oid relid, int flags, int options)
    38613861
    * to prevent schema and data changes in it. The lock level used here
    38623862
    * should match ReindexTable().
    38633863
    */
    3864-
    if ((options & REINDEXOPT_MISSING_OK) != 0)
    3864+
    if ((params->options & REINDEXOPT_MISSING_OK) != 0)
    38653865
    rel = try_table_open(relid, ShareLock);
    38663866
    else
    38673867
    rel = table_open(relid, ShareLock);
    @@ -3935,7 +3935,7 @@ reindex_relation(Oid relid, int flags, int options)
    39353935
    }
    39363936

    39373937
    reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
    3938-
    persistence, options);
    3938+
    persistence, params);
    39393939

    39403940
    CommandCounterIncrement();
    39413941

    @@ -3965,8 +3965,10 @@ reindex_relation(Oid relid, int flags, int options)
    39653965
    * Note that this should fail if the toast relation is missing, so
    39663966
    * reset REINDEXOPT_MISSING_OK.
    39673967
    */
    3968-
    result |= reindex_relation(toast_relid, flags,
    3969-
    options & ~(REINDEXOPT_MISSING_OK));
    3968+
    ReindexParams newparams = *params;
    3969+
    3970+
    newparams.options &= ~(REINDEXOPT_MISSING_OK);
    3971+
    result |= reindex_relation(toast_relid, flags, &newparams);
    39703972
    }
    39713973

    39723974
    return result;

    ‎src/backend/commands/cluster.c

    Lines changed: 11 additions & 8 deletions
    Original file line numberDiff line numberDiff line change
    @@ -103,7 +103,7 @@ void
    103103
    cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
    104104
    {
    105105
    ListCell *lc;
    106-
    int options = 0;
    106+
    ClusterParams params = {0};
    107107
    bool verbose = false;
    108108

    109109
    /* Parse option list */
    @@ -121,7 +121,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
    121121
    parser_errposition(pstate, opt->location)));
    122122
    }
    123123

    124-
    options = (verbose ? CLUOPT_VERBOSE : 0);
    124+
    params.options = (verbose ? CLUOPT_VERBOSE : 0);
    125125

    126126
    if (stmt->relation != NULL)
    127127
    {
    @@ -192,7 +192,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
    192192
    table_close(rel, NoLock);
    193193

    194194
    /* Do the job. */
    195-
    cluster_rel(tableOid, indexOid, options);
    195+
    cluster_rel(tableOid, indexOid, &params);
    196196
    }
    197197
    else
    198198
    {
    @@ -234,14 +234,16 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
    234234
    foreach(rv, rvs)
    235235
    {
    236236
    RelToCluster *rvtc = (RelToCluster *) lfirst(rv);
    237+
    ClusterParams cluster_params = params;
    237238

    238239
    /* Start a new transaction for each relation. */
    239240
    StartTransactionCommand F438 ();
    240241
    /* functions in indexes may want a snapshot set */
    241242
    PushActiveSnapshot(GetTransactionSnapshot());
    242243
    /* Do the job. */
    244+
    cluster_params.options |= CLUOPT_RECHECK;
    243245
    cluster_rel(rvtc->tableOid, rvtc->indexOid,
    244-
    options | CLUOPT_RECHECK);
    246+
    &cluster_params);
    245247
    PopActiveSnapshot();
    246248
    CommitTransactionCommand();
    247249
    }
    @@ -272,11 +274,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
    272274
    * and error messages should refer to the operation as VACUUM not CLUSTER.
    273275
    */
    274276
    void
    275-
    cluster_rel(Oid tableOid, Oid indexOid, int options)
    277+
    cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
    276278
    {
    277279
    Relation OldHeap;
    278-
    bool verbose = ((options & CLUOPT_VERBOSE) != 0);
    279-
    bool recheck = ((options & CLUOPT_RECHECK) != 0);
    280+
    bool verbose = ((params->options & CLUOPT_VERBOSE) != 0);
    281+
    bool recheck = ((params->options & CLUOPT_RECHECK) != 0);
    280282

    281283
    /* Check for user-requested abort. */
    282284
    CHECK_FOR_INTERRUPTS();
    @@ -1355,6 +1357,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
    13551357
    ObjectAddress object;
    13561358
    Oid mapped_tables[4];
    13571359
    int reindex_flags;
    1360+
    ReindexParams reindex_params = {0};
    13581361
    int i;
    13591362

    13601363
    /* Report that we are now swapping relation files */
    @@ -1412,7 +1415,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
    14121415
    pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
    14131416
    PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
    14141417

    1415-
    reindex_relation(OIDOldHeap, reindex_flags, 0);
    1418+
    reindex_relation(OIDOldHeap, reindex_flags, &reindex_params);
    14161419

    14171420
    /* Report that we are now doing clean up */
    14181421
    pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,

    0 commit comments

    Comments
     (0)
    0