Skip to content

Commit 21f9cb7

Browse files
shlomi-noachshlomi-noach
committed
optionally create distinct DROP statements
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
1 parent 38d710e commit 21f9cb7

File tree

2 files changed

+100
-29
lines changed

2 files changed

+100
-29
lines changed

‎go/vt/schemadiff/partitioning_analysis.go

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ func TemporalRangePartitioningNextRotation(createTableEntity *CreateTableEntity,
543543
// TemporalRangePartitioningRetention generates a ALTER TABLE ... DROP PARTITION diff that drops all temporal partitions
544544
// expired by given time. The function returns nil if no partitions are expired. The functions returns an error if
545545
// all partitions were to be dropped, as this is highly unlikely to be the intended action.
546-
func TemporalRangePartitioningRetention(createTableEntity *CreateTableEntity, expire time.Time) (diff *AlterTableEntityDiff, err error) {
546+
func TemporalRangePartitioningRetention(createTableEntity *CreateTableEntity, expire time.Time, distinctDiffs bool) (diffs []*AlterTableEntityDiff, err error) {
547547
analysis, err := AnalyzeTemporalRangePartitioning(createTableEntity)
548548
if err != nil {
549549
return nil, err
@@ -555,12 +555,8 @@ func TemporalRangePartitioningRetention(createTableEntity *CreateTableEntity, ex
555555
if err != nil {
556556
return nil, err
557557
}
558-
alterTable := &sqlparser.AlterTable{
559-
Table: createTableEntity.Table,
560-
PartitionSpec: &sqlparser.PartitionSpec{
561-
Action: sqlparser.DropAction,
562-
},
563-
}
558+
559+
var droppedPartitionNames sqlparser.Partitions
564560
countValueRangePartitions := 0
565561
for _, partition := range createTableEntity.TableSpec.PartitionOption.Definitions {
566562
if partition.Options == nil {
@@ -586,28 +582,50 @@ func TemporalRangePartitioningRetention(createTableEntity *CreateTableEntity, ex
586582
return nil, err
587583
}
588584
if intval <= expireIntval {
589-
alterTable.PartitionSpec.Names = append(alterTable.PartitionSpec.Names, partition.Name)
585+
droppedPartitionNames = append(droppedPartitionNames, partition.Name)
590586
}
591587
case !dt.IsZero():
592588
// Partition uses a datetime, such as in these examples:
593589
// - PARTITION p0 VALUES LESS THAN ('2021-01-01 00:00:00')
594590
// - PARTITION p0 VALUES LESS THAN (TO_DAYS('2021-01-01'))
595591
if dt.Compare(expireDatetime) <= 0 {
596-
alterTable.PartitionSpec.Names = append(alterTable.PartitionSpec.Names, partition.Name)
592+
droppedPartitionNames = append(droppedPartitionNames, partition.Name)
597593
}
598594
default:
599595
// Should never get here
600596
return nil, fmt.Errorf("unsupported partitioning in table %s", createTableEntity.Name())
601597
}
602598
}
603-
if len(alterTable.PartitionSpec.Names) == 0 {
599+
if len(droppedPartitionNames) == 0 {
604600
return nil, nil
605601
}
606-
if len(alterTable.PartitionSpec.Names) == countValueRangePartitions {
602+
if len(droppedPartitionNames) == countValueRangePartitions {
607603
// This would DROP all partitions, which is highly unlikely to be the intended action.
608604
// We reject this operation.
609605
return nil, fmt.Errorf("retention at %s would drop all partitions in table %s", expireDatetime.Format(0), createTableEntity.Name())
610606
}
611-
diff = &AlterTableEntityDiff{alterTable: alterTable, from: createTableEntity}
612-
return diff, nil
607+
if distinctDiffs {
608+
for _, name := range droppedPartitionNames {
609+
alterTable := &sqlparser.AlterTable{
610+
Table: createTableEntity.Table,
611+
PartitionSpec: &sqlparser.PartitionSpec{
612+
Action: sqlparser.DropAction,
613+
},
614+
}
615+
alterTable.PartitionSpec.Names = append(alterTable.PartitionSpec.Names, name)
616+
diff := &AlterTableEntityDiff{alterTable: alterTable, from: createTableEntity}
617+
diffs = append(diffs, diff)
618+
}
619+
} else {
620+
alterTable := &sqlparser.AlterTable{
621+
Table: createTableEntity.Table,
622+
PartitionSpec: &sqlparser.PartitionSpec{
623+
Action: sqlparser.DropAction,
624+
},
625+
}
626+
alterTable.PartitionSpec.Names = droppedPartitionNames
627+
diff := &AlterTableEntityDiff{alterTable: alterTable, from: createTableEntity}
628+
diffs = append(diffs, diff)
629+
}
630+
return diffs, nil
613631
}

‎go/vt/schemadiff/partitioning_analysis_test.go

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -733,11 +733,12 @@ func TestTemporalRangePartitioningNextRotation(t *testing.T) {
733733

734734
func TestTemporalRangePartitioningRetention(t *testing.T) {
735735
tcases := []struct {
736-
name string
737-
create string
738-
expire string
739-
expectStatement string
740-
expectErr error
736+
name string
737+
create string
738+
expire string
739+
expectStatement string
740+
expectDistinctStatements []string
741+
expectErr error
741742
}{
742743
{
743744
name: "not partitioned",
@@ -789,6 +790,9 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
789790
)`,
790791
expire: "2024-12-19 00:00:00",
791792
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`",
793+
expectDistinctStatements: []string{
794+
"ALTER TABLE `t` DROP PARTITION `p0`",
795+
},
792796
},
793797
{
794798
name: "range columns over datetime, day interval with 7 days and MAXVALUE, single partition dropped, passed threshold",
@@ -801,6 +805,9 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
801805
)`,
802806
expire: "2024-12-19 01:02:03",
803807
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`",
808+
expectDistinctStatements: []string{
809+
"ALTER TABLE `t` DROP PARTITION `p0`",
810+
},
804811
},
805812
{
806813
name: "range columns over datetime, day interval with 7 days and MAXVALUE, two partitions dropped",
@@ -813,6 +820,10 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
813820
)`,
814821
expire: "2024-12-20 00:00:00",
815822
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`, `p20241220`",
823+
expectDistinctStatements: []string{
824+
"ALTER TABLE `t` DROP PARTITION `p0`",
825+
"ALTER TABLE `t` DROP PARTITION `p20241220`",
826+
},
816827
},
817828
{
818829
name: "range columns over datetime, day interval with 7 days and MAXVALUE, two partitions dropped, passed threshold",
@@ -825,6 +836,10 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
825836
)`,
826837
expire: "2024-12-20 23:59:59",
827838
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`, `p20241220`",
839+
expectDistinctStatements: []string{
840+
"ALTER TABLE `t` DROP PARTITION `p0`",
841+
"ALTER TABLE `t` DROP PARTITION `p20241220`",
842+
},
828843
},
829844
{
830845
name: "range columns over datetime, day interval with 7 days and MAXVALUE, error dropping all partitions",
@@ -872,6 +887,9 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
872887
)`,
873888
expire: "2024-12-19 00:00:00",
874889
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`",
890+
expectDistinctStatements: []string{
891+
"ALTER TABLE `t` DROP PARTITION `p0`",
892+
},
875893
},
876894
{
877895
name: "day interval using TO_DAYS, DATETIME, drop 2 partitions",
@@ -884,6 +902,10 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
884902
)`,
885903
expire: "2024-12-20 00:00:00",
886904
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`, `p20241219`",
905+
expectDistinctStatements: []string{
906+
"ALTER TABLE `t` DROP PARTITION `p0`",
907+
"ALTER TABLE `t` DROP PARTITION `p20241219`",
908+
},
887909
},
888910
{
889911
name: "day interval using TO_DAYS, DATETIME, error dropping all partitions",
@@ -919,6 +941,9 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
919941
)`,
920942
expire: "2024-12-19 00:00:00",
921943
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`",
944+
expectDistinctStatements: []string{
945+
"ALTER TABLE `t` DROP PARTITION `p0`",
946+
},
922947
},
923948
{
924949
name: "day interval using TO_DAYS in expression, DATETIME, drop 2 partitions",
@@ -931,6 +956,10 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
931956
)`,
932957
expire: "2024-12-20 00:00:00",
933958
expectStatement: "ALTER TABLE `t` DROP PARTITION `p0`, `p20241219`",
959+
expectDistinctStatements: []string{
960+
"ALTER TABLE `t` DROP PARTITION `p0`",
961+
"ALTER TABLE `t` DROP PARTITION `p20241219`",
962+
},
934963
},
935964
{
936965
name: "day interval using TO_DAYS in expression, DATETIME, error dropping all partitions",
@@ -960,20 +989,44 @@ func TestTemporalRangePartitioningRetention(t *testing.T) {
960989
entity, err := NewCreateTableEntityFromSQL(env, tcase.create)
961990
require.NoError(t, err)
962991

963-
diff, err := TemporalRangePartitioningRetention(entity, expire)
964-
if tcase.expectErr != nil {
965-
require.Error(t, err)
966-
assert.EqualError(t, err, tcase.expectErr.Error())
967-
return
968-
}
969-
require.NoError(t, err)
992+
// Validate test input itself
970993
if tcase.expectStatement == "" {
971-
assert.Nil(t, diff)
994+
require.Empty(t, tcase.expectDistinctStatements)
972995
} else {
973-
require.NotNil(t, diff)
974-
assert.Equal(t, tcase.expectStatement, diff.CanonicalStatementString())
996+
require.NotEmpty(t, tcase.expectDistinctStatements)
975997
}
998+
t.Run("combined", func(t *testing.T) {
999+
diffs, err := TemporalRangePartitioningRetention(entity, expire, false)
1000+
if tcase.expectErr != nil {
1001+
require.Error(t, err)
1002+
assert.EqualError(t, err, tcase.expectErr.Error())
1003+
return
1004+
}
1005+
require.NoError(t, err)
1006+
if tcase.expectStatement == "" {
1007+
assert.Empty(t, diffs)
1008+
} else {
1009+
require.Len(t, diffs, 1)
1010+
assert.Equal(t, tcase.expectStatement, diffs[0].CanonicalStatementString())
1011+
}
1012+
})
1013+
t.Run("distinct", func(t *testing.T) {
1014+
diffs, err := TemporalRangePartitioningRetention(entity, expire, true)
1015+
if tcase.expectErr != nil {
1016+
require.Error(t, err)
1017+
assert.EqualError(t, err, tcase.expectErr.Error())
1018+
return
1019+
}
1020+
require.NoError(t, err)
1021+
if len(tcase.expectDistinctStatements) == 0 {
1022+
assert.Empty(t, diffs)
1023+
} else {
1024+
require.Len(t, diffs, len(tcase.expectDistinctStatements))
1025+
for i, diff := range diffs {
1026+
assert.Equal(t, tcase.expectDistinctStatements[i], diff.CanonicalStatementString())
1027+
}
1028+
}
1029+
})
9761030
})
9771031
}
978-
9791032
}

0 commit comments

Comments
 (0)