Fix MERGE16_FIXME: cleanup AO vacuum, PartitionSelector, and qual pushdown#1820
Open
lss602726449 wants to merge 9 commits into
Open
Fix MERGE16_FIXME: cleanup AO vacuum, PartitionSelector, and qual pushdown#1820lss602726449 wants to merge 9 commits into
lss602726449 wants to merge 9 commits into
Conversation
AO/AOCO tables have no per-tuple xmin/xmax -- visibility is managed via visibility map at segment level, not per-tuple transaction IDs. The freeze limits computed by vacuum_set_xid_limits are meaningless for AO tables. Worse, passing MultiXactCutoff to vac_update_relstats (vacuum) or swap_relation_files (CLUSTER) incorrectly sets relminmxid on AO tables (whose relminmxid should remain InvalidMultiXactId), causing them to unnecessarily participate in database-wide datminmxid calculation. Fix by: - vacuum_ao.c: remove vacuum_set_xid_limits call, pass Invalid values directly to vac_update_relstats - appendonlyam_handler.c / aocsam_handler.c: remove vacuum_set_xid_limits call in copy_for_cluster, return Invalid values to caller - cluster.c: relax MultiXactId assert to allow InvalidMultiXactId, and reset relminmxid to InvalidMultiXactId for AO tables (matching the existing relfrozenxid override) vacuum_set_xid_limits was a pre-PG16 API kept only for AO callers. With all callers removed, delete the function and its declaration.
The CreatePartitionPruneState() call in nodePartitionSelector.c is correct -- PartitionSelector only needs the pruning data structure, not the initial pruning and subplan map renumbering that ExecInitPartitionPruning() adds on top. Remove the incorrect FIXME. Also remove two dead declarations in execPartition.h: - ExecCreatePartitionPruneState: renamed to CreatePartitionPruneState in PG15 (commit 297daa9), declaration was never cleaned up - ExecFindInitialMatchingSubPlans: folded into ExecFindMatchingSubPlans in the same refactor, declaration was never cleaned up
The subplan check in check_output_expressions was incorrectly using UNSAFE_NOTIN_PARTITIONBY_CLAUSE, which only prevents normal pushdown but still allows the qual to be pushed as a window run condition. Subplans in output expressions should completely block pushdown in Cloudberry's distributed execution model. Add a dedicated UNSAFE_HAS_SUBPLAN flag and include it in the fully unsafe set in qual_is_pushdown_safe, so quals referencing output columns containing subplans are never pushed down.
The pgstat infrastructure is fully functional now. Remove the early return in ReportTemporaryFileUsage that disabled both temp file stats reporting and log_temp_files logging. Also restore the assert in pgstat_assert_is_up.
…c_mpp_query The commented-out code stripped write permissions from RTEs on non-root slices. This is unnecessary because InitPlan already skips permission checks on non-writer segments (execMain.c:1821). Additionally, PG16 moved requiredPerms from RTE to RTEPermissionInfo, making the original code incompatible.
The forceoverwrite check is unnecessary here because verify_dir_is_empty_or_create already handles non-empty directories before tar extraction begins.
Re-enable segwalrep/select_throttle in isolation2_schedule (verified passing) and workfile_mgr_test in singlenode_isolation2_schedule.
The pg_waldump command works correctly with appendonly WAL records. Remove start_ignore/end_ignore wrappers and update expected output to use matchsubs-masked format consistent with the isolation2 version.
lss602726449
force-pushed
the
fix-merge16-fixmes
branch
from
2ead311 to
bf057e7
Compare
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
1 participant
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.
Summary
Remove unnecessary
vacuum_set_xid_limitscalls for AO/AOCO tables: AO tables don't have per-tuple transaction IDs, so freeze/cutoff computation is meaningless. Removed the calls fromvacuum_ao.c,appendonlyam_handler.c(CLUSTER), andaocsam_handler.c(CLUSTER). Fixedcluster.cto properly resetrelminmxidfor AO tables. Deleted the now-unusedvacuum_set_xid_limitsfunction entirely.Remove incorrect FIXME in PartitionSelector: The
CreatePartitionPruneStatecall is correct for Cloudberry's join-based partition pruning (distinct fromExecInitPartitionPruningwhich adds initial pruning + subplan map renumbering). Also removed two dead declarations (ExecCreatePartitionPruneState,ExecFindInitialMatchingSubPlans) left over from PG14.Add
UNSAFE_HAS_SUBPLANflag for qual pushdown safety: Replaced the incorrect reuse ofUNSAFE_NOTIN_PARTITIONBY_CLAUSE(a "soft" unsafe flag that still allows window run conditions) with a properUNSAFE_HAS_SUBPLANflag (hard block) for output columns containing subplans. This prevents unsafe pushdown of subplan-containing quals into subqueries in Cloudberry's distributed execution model.Test plan
--enable-cassertmake installcheckregression tests passisolation2tests pass