schemadiff: temporal range partition rotation analysis and operation
#17426
Conversation
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
shlomi-noach
added
Type: Enhancement
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vitess-bot
bot
added
NeedsBackportReason
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17426 +/- ##
==========================================
+ Coverage 67.71% 67.74% +0.02%
==========================================
Files 1584 1584
Lines 254511 255019 +508
==========================================
+ Hits 172346 172765 +419
- Misses 82165 82254 +89 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
…rgument Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
|
Now fully supporting weekly rotations, including all week modes, based on either requested interval or on a parsed |
shlomi-noach
requested review from
mattlord,
harshit-gangal,
systay and
deepthi
as code owners
shlomi-noach
removed
NeedsDescriptionUpdate
Signed-off-by: Shlomi Noach <[email protected]>
go/vt/schemadiff/analysis.go
Outdated
There was a problem hiding this comment.
Renamed as partitioning_analysis.go
go/vt/schemadiff/analysis_test.go
Outdated
There was a problem hiding this comment.
Renamed as partitioning_analysis_test.go
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
go/mysql/datetime/interval.go
Outdated
| case "year": | ||
| return IntervalYear | ||
| case "quarter": | ||
| return IntervalQuarter |
There was a problem hiding this comment.
Seems like this might be worth a map in the package?
| { | ||
| // RANGE COLUMNS uses non-existent column | ||
| schema: "CREATE TABLE t (id int, i INT, PRIMARY KEY (id, i)) PARTITION BY RANGE COLUMNS (i2) (PARTITION p0 VALUES LESS THAN (1))", | ||
| expectErr: &InvalidColumnInPartitionError{Table: "t", Column: "i2"}, |
There was a problem hiding this comment.
Feels like ~ Type: "not_found" would be useful?
There was a problem hiding this comment.
Good thinking. Perhaps in a followup PR, as this would apply to a larger scope of scenarios.
go/vt/schemadiff/table.go
Outdated
| return &InvalidColumnInPartitionError{Table: c.Name(), Column: colName.String()} | ||
| } | ||
| // Validate column type | ||
| // See https://dev.mysql.com/doc/refman/8.0/en/partitioning-columns.html |
There was a problem hiding this comment.
Nit, but IMO it's best to remove the version from links when we can so that they do not become stale (w/o it it will redirect to latest GA LTS version): https://dev.mysql.com/doc/refman/en/partitioning-columns.html
| @@ -0,0 +1,731 @@ | |||
| /* | |||
| Copyright 2024 The Vitess Authors. | |||
| Col *ColumnDefinitionEntity // The column used in the RANGE expression | ||
| FuncExpr *sqlparser.FuncExpr // The function used in the RANGE expression, if any | ||
| WeekMode int // The mode used in the WEEK function, if that's what's used | ||
| MaxvaluePartition *sqlparser.PartitionDefinition // The partition that has MAXVALUE, if any |
There was a problem hiding this comment.
Nit, but MaxValuePartition would match the standard (Pascal) case.
| MaxvaluePartition *sqlparser.PartitionDefinition // The partition that has MAXVALUE, if any | ||
| HighestValueDateTime datetime.DateTime // The datetime value of the highest partition (excluding MAXVALUE) | ||
| HighestValueIntVal int64 // The integer value of the highest partition (excluding MAXVALUE) | ||
| Reason string // Why IsTemporalRangePartitioned is false |
There was a problem hiding this comment.
IMO it would be nice to couple these in a struct so that the relationship is formalized and obvious. I'm not sure if something like this is better or worse though:
type IsTemporalRangePartitioned struct {
Determination bool
Reason string
}
Fine as-is.
There was a problem hiding this comment.
keeping as is for now. No strong feelings here, but I prefer not to have structure nesting.
| if createTable.TableSpec.PartitionOption == nil { | ||
| return false | ||
| } | ||
| if createTable.TableSpec.PartitionOption.Type != sqlparser.RangeType { | ||
| return false | ||
| } | ||
| return true |
There was a problem hiding this comment.
Would this not be correct? Nitty, obviously, but it feels simpler and more concise to me.
return createTable.TableSpec.PartitionOption != nil && createTable.TableSpec.PartitionOption.Type == sqlparser.RangeType
| return nil, err | ||
| } | ||
| if !IsRangePartitioned(createTableEntity.CreateTable) { | ||
| return withReason("Table does not use PARTITION BY RANGE") |
There was a problem hiding this comment.
Nit, but it looks like these strings end up becoming errors and errors aren't supposed to use sentence capitalization or punctuation.
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Description
This PR introduces elaborate analysis and operations over temporal range partitioned tables.
Temporal range partitioned tables are tables with range partitions using any form of temporal value, ie:
PARTITION BY RANGE COLUMNS (col_name)over a single column of typeDATEorDATETIMEPARTITION BY RANGE (temporal_func(col_name))using one of several explicitly supported functions:TO_SECONDSTO_DAYSYEARWEEKYEAR8.4:UNIX_TIMESTAMPand using aTIMESTAMPcolumnschemadiffthen offers:AnalyzeTemporalRangePartitioning()returningTemporalRangePartitioningAnalysis, which determines whether the table is at all partitioned byRANGE, and can be considered as being temporal partitioned. Analysis includes the minimal rotation interval, the function used, if any, the column used, whetherMAXVALUEis used, etc.TemporalRangePartitioningNextRotation(), given a table, interval, expected ahead-of-time partitions and time reference, returning a list ofALTER TABLEstatements that prepare futuristic empty partitions, as needed.schemadiffanalyzes the actual values in the table definition, whether these areDATE,DATETIME, or product of one of the supported functions, to determine which of the ahead-of-time partitions are already covered by existing schema, and which needs to be added.The function generates either
ADD PARTITIONorREORGANIZE PARTITIONstatements, based on whetherMAXVALUEpartition is present.There's a variety of conditions to make the function return an error: if the table is not temporal range partitioned, if the interval is invalid for the table, and more.
TemporalRangePartitioningRetention(): given a table and an expiration time, this function returns a (possibly empty)ALTER TABLE DROP PARTITION ...statement dropping all expired partitions. Again,schemadiffcomputed the actual partition values whether explicitDATE,DATETIMEor product of one of the supported functions.Related Issue(s)
Checklist
Deployment Notes