Implement VACUUM FULL support for OrioleDB tables.#739
Open
Serge-sudo wants to merge 1 commit into
Open
Conversation
The operation recreates the table and all associated structures (indexes, bridges, etc.) from scratch, effectively compacting the storage. As a result, all dead tuples and empty space are removed, eliminating fragmentation and reclaiming disk space.
c8b1229 to
9512be6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds OrioleDB support for VACUUM FULL by wiring a new table-AM clustering/rebuild routine and updating the test suite to validate the behavior.
Changes:
- Implement
TableAmRoutine.relation_clusterfor OrioleDB to rebuild table metadata and indexes duringVACUUM FULL. - Remove the prior utility-command guard that rejected
VACUUM FULLon OrioleDB relations. - Add/adjust regression + testgres tests and register the new regression test in
REGRESSCHECKS.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/tableam/handler.c |
Adds orioledb_relation_cluster() and hooks it into the TableAM routine. |
src/catalog/ddl.c |
Removes the utility-layer block that previously errored on VACUUM FULL for OrioleDB tables. |
src/catalog/indices.c |
Relaxes an assertion in drop_primary_index() (now early-return) and introduces minor style change. |
test/t/vacuum_test.py |
Adds testgres coverage for VACUUM FULL on OrioleDB tables (with/without PK). |
test/t/not_supported_yet_test.py |
Updates the “not supported yet” test to expect VACUUM FULL to work. |
test/sql/vacuum_full.sql |
New regression SQL test for VACUUM FULL effects on OrioleDB internal structures. |
test/expected/vacuum_full.out |
Expected output for the new regression test. |
Makefile |
Adds vacuum_full to REGRESSCHECKS. |
ci/filter_isolation_diff.py |
Removes now-obsolete isolation diff filter for the old VACUUM FULL not supported error. |
Comments suppressed due to low confidence (4)
test/t/not_supported_yet_test.py:168
- This test switched from
VACUUM (FULL, VERBOSE)toVACUUM FULL, which reduces coverage of VACUUM option parsing/output paths. If VERBOSE is now supported for OrioleDB, consider keeping the originalVACUUM (FULL, VERBOSE)form here (and in the mixed-table case) to ensure those options are exercised too.
# VACUUM FULL works for orioledb tables
node.safe_psql("""
VACUUM FULL o_test_1;
""")
# VACUUM FULL works for mixed table lists (orioledb + heap)
node.safe_psql("""
VACUUM FULL pg_test_1, o_test_1;
""")
test/sql/vacuum_full.sql:4
- The script creates a dedicated schema (
CREATE SCHEMA orioledb;) and installspageinspect, but it never drops the schema or thepageinspectextension. Other regression SQL files typically clean up withDROP EXTENSION ...andDROP SCHEMA ...at the end; please add corresponding cleanup here (and/or use a test-specific schema name + set search_path) to avoid leaving objects behind when tests are run individually or re-ordered.
CREATE SCHEMA orioledb;
CREATE EXTENSION orioledb SCHEMA orioledb;
CREATE EXTENSION pageinspect;
src/tableam/handler.c:1035
verboseis currently unused inorioledb_relation_cluster(), which can trigger unused-parameter warnings in builds. Either use it (e.g., emit progress logging when verbose is true) or explicitly mark it unused.
static void
orioledb_relation_cluster(Relation tbl, bool verbose)
{
test/t/vacuum_test.py:243
- The test comment says the secondary index still works after VACUUM FULL, but the query filters on the primary key (val_1 = 42) and doesn’t exercise the newly-created secondary index on val_2. Consider changing the assertion to force/use the secondary index (e.g., query by val_2 with seqscan disabled or assert the plan uses the index) so the test actually validates secondary-index rebuild behavior.
# Index still works after VACUUM FULL
result = node.execute(
"SELECT val_2 FROM o_test_1 WHERE val_1 = 42;")
self.assertEqual(result[0][0], 52)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The operation recreates the table and all associated structures (indexes, bridges, etc.) from scratch, effectively compacting the storage. As a result, all dead tuples and empty space are removed, eliminating fragmentation and reclaiming disk space.
uplink -> orioledb/postgres#48