MDEV-25529 Auto-create: Pre-existing historical data is not partitioned as specified by ALTER#5294
MDEV-25529 Auto-create: Pre-existing historical data is not partitioned as specified by ALTER#5294midenok wants to merge 8 commits into
Conversation
* get_next_time() comment * THD::used comment
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the ALTER TABLE FORCE syntax and implements automatic partitioning of pre-existing historical data for system-versioned tables (MDEV-25529). Key changes include adding high-precision timestamp helpers, updating parser rules to allow flexible ordering of partitioning clauses, and introducing logic to calculate the required history partitions based on the table's actual historical time range. The feedback suggests renaming the generic cmp function in my_time.h to avoid global namespace collisions, removing a temporary TODO comment, initializing best_idx to prevent compiler warnings, and simplifying the partition calculation logic in vers_calc_hist_parts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR addresses MDEV-25529 by ensuring that when ALTER TABLE ... PARTITION BY SYSTEM_TIME ... AUTO is used on a system-versioned table, pre-existing historical rows are accounted for when auto-creating history partitions (including adjusting STARTS when needed). It also expands ALTER TABLE grammar around FORCE + partitioning clauses and adds supporting timestamp/time utilities plus new regression tests.
Changes:
- Derive historical
ROW ENDtimestamp range (index-first, scan fallback) and use it during versioning partition auto-setup on ALTER to size partitions and/or adjustSTARTS. - Add
my_timespec_tand session-timezone timestamp formatting helpers used in warnings/debug paths. - Extend parser grammar for more
ALTER TABLEclause orderings and add/refresh mysql-test coverage for both MDEV-25529 behavior and FORCE syntax.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
sql/table.h |
Declares TABLE::vers_get_history_range() for versioned-table history range discovery. |
sql/sql_yacc.yy |
Extends ALTER TABLE grammar to allow additional orderings for partitioning/remove partitioning with other alter clauses. |
sql/sql_type.h |
Declares Temporal_hybrid(THD*, my_timespec_t) ctor for high-precision timestamps. |
sql/sql_type.cc |
Implements Temporal_hybrid(THD*, my_timespec_t) conversion via session time zone. |
sql/sql_time.h |
Adds interval2sec() / interval2usec() helpers to replace macro-based interval math. |
sql/sql_time.cc |
Switches interval microsecond calculations to the new helpers. |
sql/sql_table.cc |
Minor refactor in duplicate-key check predicate naming; adds include needed for key utilities. |
sql/sql_partition.cc |
Main logic: on relevant ALTER, compute history range, adjust STARTS, and compute needed history partitions (incl. month/year stepping). |
sql/sql_class.h |
Adds THD::timestamp_to_string() overloads and TimestampString helper for lazy formatting. |
sql/sql_class.cc |
Implements THD::timestamp_to_string() using Temporal_hybrid and marks TIME_ZONE_USED. |
sql/share/errmsg-utf8.txt |
Reworks/introduces warnings related to STARTS and slow history scanning; adds new warning messages. |
sql/partition_info.h |
Tracks whether STARTS was explicitly provided; adds vers_set_starts() API. |
sql/partition_info.cc |
Implements vers_set_starts() rounding; improves interval warning message; adds OOM error on default partition element allocation. |
sql/field.h |
Adds Field_vers_trx_id::get_timestamp() override declaration. |
sql/field.cc |
Implements transaction-registry timestamp lookup for TRX_ID-versioned fields. |
sql/event_data_objects.cc |
Expands documentation comment for interval scheduling behavior. |
mysql-test/suite/versioning/t/partition2.test |
Adds comprehensive regression test for MDEV-25529 (index vs scan paths, STARTS adjustments, month/year intervals, etc.). |
mysql-test/suite/versioning/r/partition2.result |
Expected output for the new versioning partition regression test. |
mysql-test/suite/versioning/r/partition.result |
Updates expected warnings text for STARTS-beyond-query messages. |
mysql-test/main/partition.test |
Adds FORCE syntax regression coverage for ALTER TABLE partitioning/remove partitioning. |
mysql-test/main/partition.result |
Expected output for new FORCE syntax test cases. |
include/my_time.h |
Introduces my_timespec_t, ordering operators, and min/max constants for high-precision timestamps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
midenok
force-pushed
the
main-midenok-MDEV-25529
branch
from
b53bf6a to
e49a9c5
Compare
It returned non-NULL on alloc error.
Improves ALTER TABLE syntax when alter_list can be supplied alongside a partitioning expression, so that they can appear in any order. This is particularly useful for the FORCE clause when adding it to an existing command. Also improves handling of AUTO with FORCE, so that AUTO FORCE specified together provides more consistent syntax, which is used by this task in further commits.
…ed as specified by ALTER Adds logic into prep_alter_part_table() for AUTO to check the history range (vers_get_history_range()) and based on (max_ts - min_ts) difference compute the number of created partitions and set STARTS value to round down min_ts value (vers_set_starts()) if it was not specified by user or if the user specified it incorrectly. In the latter case it will print warning about wrongly specified user value. In case of fast ALTER TABLE, f.ex. when partitioning already exists, the above logic is ignored unless FORCE clause is specified. When user specifies partition list explicitly the above logic is ignored even with FORCE clause. vers_get_history_range() detects if the index can be used for row_end min/max stats and if so it gets it with ha_index_first() and HA_READ_BEFORE_KEY (as it must ignore current data). Otherwise it does table scan to read the stats. There is test_mdev-25529 debug keyword to check the both and compare results. A warning is printed if the algorithm uses slow scan. Field_vers_trx_id::get_timestamp() is implemented for TRX_ID based versioning to get epoch value. It works in vers_get_history_range() but since partitioning is not enabled for TRX_ID versioning create temporary table fails with error, requiring timestamp-based system fields. This method will be useful when partitioning will be enabled for TRX_ID which is mostly performance problems to solve. Static key_cmp was renamed to key_eq to resolve compilation after key.h was included as key_cmp was already declared there.
midenok
force-pushed
the
main-midenok-MDEV-25529
branch
from
419c5cd to
74ccf25
Compare
| if (table->versioned() && | ||
| (alter_info->partition_flags & ALTER_PARTITION_INFO) && | ||
| part_info->part_type == VERSIONING_PARTITION && |
| DBUG_ASSERT(versioned()); | ||
| // Find best key | ||
| KEY *key= key_info, *best_key= NULL; | ||
| uint best_idx; | ||
| Field *end_field= vers_end_field(); |
Please, review.