Derive commit query shape from table partitioning
What does this MR do and why?
This MR refines the mr_diff_commits_read_new_table feature flag to guard code based on both flag state AND table partitioning status. This ensures query shapes only change after the table swap (!244091 (merged)), when the data structure supports them.
Key changes:
- Query shape now depends on table partitioning state, not just the feature flag
- Adds a catalog check to verify partitioning status (temporary tradeoff accepted to align query timing with table structure)
- Removes the
mr_diff_commits_project_id_pruningflag — this optimization can now be enabled alongsidemr_diff_commits_read_new_tableonce the swap completes
Why this matters: The new query shapes require the swapped table structure. By tying queries to partitioning state rather than just the flag, we prevent query mismatches with the current data state.
References
- https://gitlab.com/gitlab-org/gitlab/-/work_items/527241
- https://gitlab.com/gitlab-org/gitlab/-/work_items/602841#note_3684611191
Database
No query text changes in this MR. Both shapes already exist in production and are unchanged; only the condition selecting between them moves from a flag to the table's partitioning.MergeRequestDiffCommit.read_new_commits_table? is now the single decision point for shape and for pruning.
How to set up and validate locally
To validate that the partitioning check correctly gates the query shape change, simulate the table swap locally by renaming tables inside a transaction.
What to expect: The partitioned? check should flip from false to true when tables are swapped. Both commit_shas and commits calls should succeed before and after the swap - if the guard weren't working, commits would raise ActiveModel::MissingAttributeError after the swap (the new table lacks commit_author/committer columns).
Setup and test
Feature.enable(:mr_diff_commits_read_new_table)
mr_id = MergeRequest.joins(:merge_request_diffs).where.not(latest_merge_request_diff_id: nil).first.id
def report(label, mr_id)
mr = MergeRequest.find(mr_id) # re-fetch: the predicate is memoised per instance
diff = mr.latest_merge_request_diff
puts "#{label} partitioned? #{MergeRequestDiffCommit.read_new_commits_table?(mr.target_project_id)}"
puts "#{label} commit_shas #{diff.commit_shas(limit: 2, mode: :force_metadata).size}"
puts "#{label} commits #{diff.commits(limit: 2).size}"
rescue StandardError => e
puts "#{label} RAISED #{e.class}: #{e.message.lines.first.to_s.strip}"
end
# Before swap
report('before', mr_id)
# Simulate swap: rename tables and observe behavior
ActiveRecord::Base.transaction do
c = ActiveRecord::Base.connection
c.execute('ALTER TABLE merge_request_diff_commits RENAME TO mrdc_legacy_tmp')
c.execute('ALTER TABLE merge_request_diff_commits_b5377a7a34 RENAME TO merge_request_diff_commits')
report('after ', mr_id)
raise ActiveRecord::Rollback
endSpecs: Tests stub read_new_commits_table? rather than the flag and partition check separately, so they describe the two states that matter (legacy vs. new table) and won't need updating when the flag is removed. Edge cases like disabling the flag while the table is partitioned are covered in merge_request_diff_commit_spec.rb.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Related to #527241