Skip to content
This repository was archived by the owner on Nov 7, 2025. It is now read-only.

Commit 0cca9aa

Browse files
mieciumieciu
authored
Regression fix for CREATE TABLE ... ON CLUSTER (#1491)
We have recently introduced a regression which could result in table creation failures whenever `ON CLUSTER` clause had to be applied.
1 parent 5b2cf47 commit 0cca9aa

File tree

8 files changed

+149
-569
lines changed

8 files changed

+149
-569
lines changed

‎platform/database_common/log_manager_test.go‎

Lines changed: 0 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
schema2 "github.com/QuesmaOrg/quesma/platform/schema"
99
"github.com/QuesmaOrg/quesma/platform/types"
1010
"github.com/QuesmaOrg/quesma/platform/util"
11-
"strings"
1211
"sync/atomic"
1312
"testing"
1413

@@ -465,161 +464,6 @@ func TestJsonConvertingBoolToStringAttr(t *testing.T) {
465464
}
466465
}
467466

468-
// Doesn't test for 100% equality, as map iteration order isn't deterministic, but should definitely be good enough.
469-
func TestCreateTableString_1(t *testing.T) {
470-
table := Table{
471-
Name: "/_bulk?refresh=false&_source_includes=originId&require_alias=true_16",
472-
Cols: map[string]*Column{
473-
"doc": {
474-
Name: "doc",
475-
Type: MultiValueType{
476-
Name: "Tuple",
477-
Cols: []*Column{
478-
{
479-
Name: "Tuple",
480-
Type: MultiValueType{
481-
Name: "Tuple",
482-
Cols: []*Column{
483-
{
484-
Name: "runAt",
485-
Type: NewBaseType("DateTime64"),
486-
},
487-
{
488-
Name: "startedAt",
489-
Type: NewBaseType("DateTime64"),
490-
},
491-
{
492-
Name: "Tuple",
493-
Type: NewBaseType("String"),
494-
},
495-
{
496-
Name: "status",
497-
Type: NewBaseType("String"),
498-
},
499-
},
500-
},
501-
},
502-
{
503-
Name: "updated_at",
504-
Type: NewBaseType("DateTime64"),
505-
},
506-
},
507-
},
508-
},
509-
"@timestamp": {
510-
Name: "@timestamp",
511-
Type: NewBaseType("DateTime64"),
512-
},
513-
},
514-
Config: &ChTableConfig{
515-
HasTimestamp: true,
516-
TimestampDefaultsNow: true,
517-
Engine: "MergeTree",
518-
OrderBy: "(@timestamp)",
519-
PrimaryKey: "",
520-
Ttl: "",
521-
Attributes: []Attribute{
522-
NewDefaultStringAttribute(),
523-
},
524-
CastUnsupportedAttrValueTypesToString: false,
525-
PreferCastingToOthers: false,
526-
},
527-
}
528-
expectedRows := []string{
529-
`CREATE TABLE IF NOT EXISTS "/_bulk?refresh=false&_source_includes=originId&require_alias=true_16" (`,
530-
`"doc" Tuple`,
531-
`(`,
532-
`"Tuple" Tuple`,
533-
`(`,
534-
`"runAt" DateTime64,`,
535-
`"startedAt" DateTime64,`,
536-
`"Tuple" String,`,
537-
`"status" String`,
538-
`),`,
539-
`"updated_at" DateTime64`,
540-
`),`,
541-
`"@timestamp" DateTime64,`,
542-
`"attributes_values" Map(String,String),`,
543-
`"attributes_metadata" Map(String,String)`,
544-
`)`,
545-
`ENGINE = MergeTree`,
546-
`ORDER BY (@timestamp)`,
547-
"",
548-
}
549-
createTableString := table.CreateTableString()
550-
for _, row := range strings.Split(createTableString, "\n") {
551-
assert.Contains(t, expectedRows, strings.TrimSpace(row))
552-
}
553-
}
554-
555-
// Doesn't test for 100% equality, as map iteration order isn't deterministic, but should definitely be good enough.
556-
func TestCreateTableString_NewDateTypes(t *testing.T) {
557-
table := Table{
558-
Name: "abc",
559-
Cols: map[string]*Column{
560-
"low_card_string": {
561-
Name: "low_card_string",
562-
Type: NewBaseType("LowCardinality(String)"),
563-
Comment: "some comment 1",
564-
},
565-
"uuid": {
566-
Name: "uuid",
567-
Type: NewBaseType("UUID"),
568-
},
569-
"int32": {
570-
Name: "int32",
571-
Type: NewBaseType("Int32"),
572-
Comment: "some comment 2",
573-
},
574-
"epoch_time": {
575-
Name: "epoch_time",
576-
Type: NewBaseType("DateTime('Asia/Kolkata')"),
577-
Modifiers: "CODEC(DoubleDelta, LZ4)",
578-
},
579-
"estimated_connection_speedinkbps": {
580-
Name: "estimated_connection_speedinkbps",
581-
Type: NewBaseType("Float64"),
582-
Modifiers: "CODEC(DoubleDelta, LZ4)",
583-
},
584-
},
585-
Config: &ChTableConfig{
586-
HasTimestamp: true,
587-
TimestampDefaultsNow: true,
588-
Engine: "MergeTree",
589-
OrderBy: "(@timestamp)",
590-
PrimaryKey: "",
591-
Ttl: "",
592-
Attributes: []Attribute{
593-
NewDefaultInt64Attribute(),
594-
},
595-
CastUnsupportedAttrValueTypesToString: true,
596-
PreferCastingToOthers: true,
597-
},
598-
}
599-
expectedRows := []string{
600-
`CREATE TABLE IF NOT EXISTS "abc" (`,
601-
`"int32" Int32 COMMENT 'some comment 2',`,
602-
`"low_card_string" LowCardinality(String) COMMENT 'some comment 1',`,
603-
`"uuid" UUID,`,
604-
`"others" JSON,`,
605-
`"attributes_int64_key" Array(String),`,
606-
`"attributes_int64_value" Array(Int64),`,
607-
`"attributes_values" Map(String,String),`,
608-
`"attributes_metadata" Map(String,String)`,
609-
`"@timestamp" DateTime64(3) DEFAULT now64(),`,
610-
`"epoch_time" DateTime('Asia/Kolkata') CODEC(DoubleDelta, LZ4),`,
611-
`"estimated_connection_speedinkbps" Float64 CODEC(DoubleDelta, LZ4),`,
612-
`ENGINE = MergeTree`,
613-
`)`,
614-
`ORDER BY (@timestamp)`,
615-
"",
616-
}
617-
createTableString := table.CreateTableString()
618-
for _, row := range strings.Split(createTableString, "\n") {
619-
assert.Contains(t, expectedRows, strings.TrimSpace(row))
620-
}
621-
}
622-
623467
func TestLogManager_GetTable(t *testing.T) {
624468
tests := []struct {
625469
name string

‎platform/database_common/table.go‎

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"github.com/QuesmaOrg/quesma/platform/config"
99
"github.com/QuesmaOrg/quesma/platform/logger"
1010
"github.com/QuesmaOrg/quesma/platform/model"
11-
"github.com/QuesmaOrg/quesma/platform/util"
1211
"strconv"
1312
"strings"
1413
)
@@ -30,48 +29,6 @@ type Table struct {
3029
ExistsOnAllNodes bool
3130
}
3231

33-
func (t *Table) createTableOurFieldsString() []string {
34-
rows := make([]string, 0)
35-
if t.Config.HasTimestamp {
36-
_, ok := t.Cols[timestampFieldName]
37-
if !ok {
38-
defaultStr := ""
39-
if t.Config.TimestampDefaultsNow {
40-
defaultStr = " DEFAULT now64()"
41-
}
42-
rows = append(rows, fmt.Sprintf("%s\"%s\" DateTime64(3)%s", util.Indent(1), timestampFieldName, defaultStr))
43-
}
44-
}
45-
if len(t.Config.Attributes) > 0 {
46-
for _, a := range t.Config.Attributes {
47-
_, ok := t.Cols[a.MapValueName]
48-
if !ok {
49-
rows = append(rows, fmt.Sprintf("%s\"%s\" Map(String,String)", util.Indent(1), a.MapValueName))
50-
}
51-
_, ok = t.Cols[a.MapMetadataName]
52-
if !ok {
53-
rows = append(rows, fmt.Sprintf("%s\"%s\" Map(String,String)", util.Indent(1), a.MapMetadataName))
54-
}
55-
56-
}
57-
}
58-
return rows
59-
}
60-
61-
func (t *Table) CreateTableString() string {
62-
var onClusterClause string
63-
if t.ClusterName != "" {
64-
onClusterClause = " ON CLUSTER " + strconv.Quote(t.ClusterName)
65-
}
66-
s := "CREATE TABLE IF NOT EXISTS " + t.FullTableName() + onClusterClause + " (\n"
67-
rows := make([]string, 0)
68-
for _, col := range t.Cols {
69-
rows = append(rows, col.createTableString(1))
70-
}
71-
rows = append(rows, t.createTableOurFieldsString()...)
72-
return s + strings.Join(rows, ",\n") + "\n)\n" + t.Config.CreateTablePostFieldsString()
73-
}
74-
7532
// FullTableName returns full table name with database name if it's not empty.
7633
// Format: ["database".]"table" as it seems to work for all cases in Clickhouse.
7734
// Use this in Clickhouse queries, e.g. in FROM clause.

‎platform/ingest/ast.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (ct CreateTableStatement) ToSQL() string {
5353
var b strings.Builder
5454

5555
if ct.Cluster != "" {
56-
b.WriteString(fmt.Sprintf(`CREATE TABLE IF NOT EXISTS "%s" ON CLUSTER "%s"`+" \n(\n\n", ct.Name, ct.Cluster))
56+
b.WriteString(fmt.Sprintf(`CREATE TABLE IF NOT EXISTS "%s" ON CLUSTER "%s"`+" \n", ct.Name, ct.Cluster))
5757
} else {
5858
b.WriteString(fmt.Sprintf(`CREATE TABLE IF NOT EXISTS "%s"`, ct.Name))
5959
}

‎platform/ingest/ast_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright Quesma, licensed under the Elastic License 2.0.
2+
// SPDX-License-Identifier: Elastic-2.0
3+
4+
package ingest
5+
6+
import (
7+
"github.com/stretchr/testify/assert"
8+
"testing"
9+
)
10+
11+
func TestCreateTableStatement_ToSQL(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
stmt CreateTableStatement
15+
expected string
16+
}{
17+
{
18+
name: "basic table",
19+
stmt: CreateTableStatement{
20+
Name: "my_table",
21+
Columns: []ColumnStatement{
22+
{ColumnName: "id", ColumnType: "Int64"},
23+
{ColumnName: "@timestamp", ColumnType: "DateTime64(3)", AdditionalMetadata: "DEFAULT now64()"},
24+
{ColumnName: "name", ColumnType: "String", Comment: "user name"},
25+
},
26+
Comment: "created by Quesma",
27+
PostClause: "ENGINE = MergeTree() ORDER BY (\"@timestamp\")",
28+
},
29+
expected: `CREATE TABLE IF NOT EXISTS "my_table"
30+
(
31+
32+
"id" Int64,
33+
"@timestamp" DateTime64(3) DEFAULT now64(),
34+
"name" String COMMENT 'user name'
35+
)
36+
ENGINE = MergeTree() ORDER BY ("@timestamp")
37+
COMMENT 'created by Quesma'`,
38+
},
39+
{
40+
name: "basic table with cluster",
41+
stmt: CreateTableStatement{
42+
Name: "my_table",
43+
Cluster: "quesma_cluster",
44+
Columns: []ColumnStatement{
45+
{ColumnName: "id", ColumnType: "Int64"},
46+
},
47+
Comment: "created by Quesma",
48+
PostClause: "ENGINE = MergeTree() ORDER BY (\"id\")",
49+
},
50+
expected: `CREATE TABLE IF NOT EXISTS "my_table" ON CLUSTER "quesma_cluster"
51+
52+
(
53+
54+
"id" Int64
55+
)
56+
ENGINE = MergeTree() ORDER BY ("id")
57+
COMMENT 'created by Quesma'`,
58+
},
59+
}
60+
61+
for _, tt := range tests {
62+
t.Run(tt.name, func(t *testing.T) {
63+
got := tt.stmt.ToSQL()
64+
assert.Equal(t, tt.expected, got)
65+
})
66+
}
67+
}

0 commit comments

Comments
 (0)