Fix MERGE16_FIXME: cleanup AO vacuum, PartitionSelector, and qual pushdown#1820
Open
lss602726449 wants to merge 3 commits into
Open
Fix MERGE16_FIXME: cleanup AO vacuum, PartitionSelector, and qual pushdown#1820lss602726449 wants to merge 3 commits into
lss602726449 wants to merge 3 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.
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