--partitions=(v1,'v2') could calculate wrong partition expression i…

…f `system.columns` will return fields in different order than they described in PARTITION BY clause, fix #791 for embedded BACKUP
Altinity · Nov 24, 2023 · 53893c1 · 53893c1
1 parent b66908c
commit 53893c1
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 2 deletions.
diff --git a/pkg/backup/create.go b/pkg/backup/create.go
@@ -291,7 +291,15 @@ func (b *Backuper) createBackupEmbedded(ctx context.Context, backupName, tablePa
 		tablesSQL += "TABLE `" + table.Database + "`.`" + table.Name + "`"
 		tableSizeSQL += "'" + table.Database + "." + table.Name + "'"
 		if nameList, exists := partitionsNameList[metadata.TableTitle{Database: table.Database, Table: table.Name}]; exists && len(nameList) > 0 {
-			tablesSQL += fmt.Sprintf(" PARTITIONS '%s'", strings.Join(nameList, "','"))
+			partitionsSQL := ""
+			for _, partitionName := range nameList {
+				if strings.HasPrefix(partitionName, "(") {
+					partitionsSQL += partitionName + ","
+				} else {
+					partitionsSQL += "'" + partitionName + "',"
+				}
+			}
+			tablesSQL += fmt.Sprintf(" PARTITIONS %s", partitionsSQL[:len(partitionsSQL)-1])
 		}
 		if i < l {
 			tablesSQL += ", "

diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go
@@ -2032,7 +2032,7 @@ func testBackupSpecifiedPartitions(t *testing.T, r *require.Assertions, ch *Test
 	r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", "/etc/clickhouse-backup/"+backupConfig, "delete", "local", fullBackupName))
 
 	// check create + partitions
-	r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", "/etc/clickhouse-backup/"+backupConfig, "create", "--tables="+dbName+".t1", "--partitions=0-20220102,0-20220103", partitionBackupName))
+	r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", "/etc/clickhouse-backup/"+backupConfig, "create", "--tables="+dbName+".t1", "--partitions=(0,'2022-01-02'),(0,'2022-01-03')", partitionBackupName))
 	partitionBackupDir := "/var/lib/clickhouse/backup/" + partitionBackupName + "/shadow/" + dbName + "/t1/default/"
 	if strings.HasPrefix(remoteStorageType, "EMBEDDED") {
 		partitionBackupDir = "/var/lib/clickhouse/disks/backups" + strings.ToLower(strings.TrimPrefix(remoteStorageType, "EMBEDDED")) + "/" + partitionBackupName + "/data/" + dbName + "/t1"