Skip to content

Conversation

@m3hm3t
Copy link
Contributor

@m3hm3t m3hm3t commented Nov 28, 2025

fixes #8364

PostgreSQL 18 changes VACUUM/ANALYZE to recurse into inheritance children by default, and introduces ONLY to limit processing to the parent. Upstream change:
postgres/postgres@62ddf7ee9

For Citus tables, we should treat shard placements as “children” and avoid propagating VACUUM/ANALYZE to shards when the user explicitly asks for ONLY.

This PR adjusts the Citus VACUUM handling to align with PG18 semantics, and adds regression coverage on both regular distributed tables and partitioned distributed tables.


Behavior changes

  • Introduce a per-relation helper struct:

    typedef struct CitusVacuumRelation
    {
        VacuumRelation *vacuumRelation;
        Oid             relationId;
    } CitusVacuumRelation;

    This lets us keep both:

    • the resolved relation OID (for IsCitusTable, task building), and
    • the original VacuumRelation node (for column list and ONLY/inh flag).
  • Replace the old VacuumRelationIdList / ExtractVacuumTargetRels flow with:

    static List *VacuumRelationList(VacuumStmt *vacuumStmt,
                                    CitusVacuumParams vacuumParams);

    VacuumRelationList now:

    • Iterates over vacuumStmt->rels.
    • Resolves relid via RangeVarGetRelidExtended when relation is present.
    • Falls back to locking VacuumRelation->oid when only an OID is available.
    • Respects VACOPT_FULL for lock mode and VACOPT_SKIP_LOCKED for locking behavior.
    • Builds a List * of CitusVacuumRelation entries.
  • Update:

    IsDistributedVacuumStmt(List *vacuumRelationList);
    ExecuteVacuumOnDistributedTables(VacuumStmt *vacuumStmt,
                                     List *vacuumRelationList,
                                     CitusVacuumParams vacuumParams);

    to operate on CitusVacuumRelation instead of bare OIDs.

  • Implement ONLY semantics in ExecuteVacuumOnDistributedTables:

    RangeVar *relation = vacuumRelation->relation;
    if (relation != NULL && !relation->inh)
    {
        /* ONLY specified, so don't recurse to shard placements */
        continue;
    }

    Effect:

    • VACUUM / ANALYZE (no ONLY) on a Citus table: behavior unchanged, Citus creates tasks and propagates to shard placements.

    • VACUUM ONLY <citus_table> / ANALYZE ONLY <citus_table>:

      • Core still processes the coordinator relation as usual.
      • Citus skips building tasks for shard placements, so we do not recurse into distributed children.
    • The code compiles and behaves as before on pre-PG18; the new behavior becomes observable only when the core planner starts setting inh = false for ONLY (PG18).

  • Unqualified VACUUM / ANALYZE (no rels) is unchanged and still handled via ExecuteUnqualifiedVacuumTasks.

  • Remove now-redundant helpers:

    • VacuumColumnList
    • ExtractVacuumTargetRels

    Column lists are now taken directly from vacuumRelation->va_cols via CitusVacuumRelation.


Testing

Extend src/test/regress/sql/pg18.sql and expected/pg18.out with two PG18-only blocks that verify we do not recurse into shard placements when ONLY is used:

  1. Simple distributed table (pg18_vacuum_part)

    • Create and distribute a regular table:

      CREATE SCHEMA pg18_vacuum_part;
      SET search_path TO pg18_vacuum_part;
      CREATE TABLE vac_analyze_only (a int);
      SELECT create_distributed_table('vac_analyze_only', 'a');
      INSERT INTO vac_analyze_only VALUES (1), (2), (3);
    • On the coordinator:

      • Run ANALYZE vac_analyze_only; and later ANALYZE ONLY vac_analyze_only;.
      • Run VACUUM vac_analyze_only; and later VACUUM ONLY vac_analyze_only;.
    • On worker_1:

      • Capture coalesce(max(last_analyze), 'epoch') from pg_stat_user_tables for vac_analyze_only_% into :analyze_before_only, then assert:

        SELECT max(last_analyze) = :'analyze_before_only'::timestamptz AS analyze_only_skipped;
      • Capture coalesce(max(last_vacuum), 'epoch') into :vacuum_before_only, then assert:

        SELECT max(last_vacuum) = :'vacuum_before_only'::timestamptz AS vacuum_only_skipped;

      Both checks return t, confirming ONLY does not change last_analyze / last_vacuum on shard tables.

  2. Partitioned distributed table (pg18_vacuum_part_dist)

    • Create a partitioned table whose parent is distributed:

      CREATE SCHEMA pg18_vacuum_part_dist;
      SET search_path TO pg18_vacuum_part_dist;
      SET citus.shard_count = 2;
      SET citus.shard_replication_factor = 1;
      
      CREATE TABLE part_dist (id int, v int) PARTITION BY RANGE (id);
      CREATE TABLE part_dist_1 PARTITION OF part_dist FOR VALUES FROM (1) TO (100);
      CREATE TABLE part_dist_2 PARTITION OF part_dist FOR VALUES FROM (100) TO (200);
      
      SELECT create_distributed_table('part_dist', 'id');
      INSERT INTO part_dist SELECT g, g FROM generate_series(1, 199) g;
    • On the coordinator:

      • Run ANALYZE part_dist; then ANALYZE ONLY part_dist;.
      • Run VACUUM part_dist; then VACUUM ONLY part_dist; (PG18 emits the expected warning: VACUUM ONLY of partitioned table "part_dist" has no effect).
    • On worker_1:

      • Capture coalesce(max(last_analyze), 'epoch') for part_dist_% into :analyze_before_only, then assert:

        SELECT max(last_analyze) = :'analyze_before_only'::timestamptz
               AS analyze_only_partitioned_skipped;
      • Capture coalesce(max(last_vacuum), 'epoch') into :vacuum_before_only, then assert:

        SELECT max(last_vacuum) = :'vacuum_before_only'::timestamptz
               AS vacuum_only_partitioned_skipped;

      Both checks return t, confirming that even for a partitioned distributed parent, VACUUM/ANALYZE ONLY does not recurse into shard placements, and Citus behavior matches PG18’s “ONLY = parent only” semantics.

@m3hm3t m3hm3t self-assigned this Nov 28, 2025
@m3hm3t m3hm3t changed the title Add support for VACUUM/ANALYZE to recurse into inheritance children b… PG18 – Respect VACUUM/ANALYZE ONLY semantics for Citus tables Dec 1, 2025
@m3hm3t m3hm3t marked this pull request as ready for review December 1, 2025 09:15
@m3hm3t m3hm3t requested review from colm-mchugh and naisila December 1, 2025 09:15
@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_beta_confs branch 2 times, most recently from 1d606af to c499b6b Compare December 1, 2025 12:30
@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_vacuum_feature branch from efb93ff to 321d1d5 Compare December 1, 2025 12:32
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 73.52941% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.93%. Comparing base (3399d66) to head (02af7f8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8365      +/-   ##
==========================================
- Coverage   88.94%   88.93%   -0.02%     
==========================================
  Files         287      287              
  Lines       63146    63150       +4     
  Branches     7935     7938       +3     
==========================================
- Hits        56164    56160       -4     
- Misses       4672     4677       +5     
- Partials     2310     2313       +3     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_beta_confs branch from c499b6b to 459128f Compare December 5, 2025 08:34
@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_vacuum_feature branch from 321d1d5 to ac691b1 Compare December 5, 2025 08:35
@m3hm3t m3hm3t requested a review from colm-mchugh December 5, 2025 09:18
Copy link
Contributor

@colm-mchugh colm-mchugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, thanks for including the test on a distributed, partitioned table.

@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_beta_confs branch from 459128f to bdd4c24 Compare December 5, 2025 12:54
@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_vacuum_feature branch from 76f8090 to d1b96e8 Compare December 5, 2025 12:55
@m3hm3t m3hm3t changed the base branch from m3hm3t/pg18_beta_confs to main December 5, 2025 13:18
@m3hm3t m3hm3t force-pushed the m3hm3t/pg18_vacuum_feature branch from d1b96e8 to 02af7f8 Compare December 5, 2025 13:19
@m3hm3t m3hm3t merged commit 31911d8 into main Dec 5, 2025
153 of 154 checks passed
@m3hm3t m3hm3t deleted the m3hm3t/pg18_vacuum_feature branch December 5, 2025 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ONLY support for VACUUM and ANALYZE

3 participants