Skip to content

Commit 6453099

Browse files
begelundmullerbegelundmuller
authored
Ensure correct case handling of time dimensions (#8448)
1 parent 9ca76f4 commit 6453099

File tree

3 files changed

+99
-7
lines changed

3 files changed

+99
-7
lines changed

‎runtime/metricsview/executor/executor_summary.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (e *Executor) Summary(ctx context.Context) (*SummaryResult, error) {
6161
if e.metricsView.TimeDimension != "" {
6262
var found bool
6363
for _, dim := range timeDimensions {
64-
if dim.Name == e.metricsView.TimeDimension {
64+
if strings.EqualFold(dim.Name, e.metricsView.TimeDimension) {
6565
found = true
6666
break
6767
}
@@ -106,7 +106,7 @@ func (e *Executor) Summary(ctx context.Context) (*SummaryResult, error) {
106106
summary.MaxValue = timeRange.Max
107107
}
108108
summaries = append(summaries, summary)
109-
if dim.Name == e.metricsView.TimeDimension {
109+
if strings.EqualFold(dim.Name, e.metricsView.TimeDimension) {
110110
defaultTimeDimensionSummary = summary
111111
}
112112
}
@@ -140,7 +140,7 @@ func (e *Executor) Summary(ctx context.Context) (*SummaryResult, error) {
140140
// Determine the default time dimension expression
141141
var timeDimExpr string
142142
for _, dim := range timeDimensions {
143-
if dim.Name == e.metricsView.TimeDimension {
143+
if strings.EqualFold(dim.Name, e.metricsView.TimeDimension) {
144144
expr, err := e.olap.Dialect().MetricsViewDimensionExpression(dim)
145145
if err == nil {
146146
timeDimExpr = expr

‎runtime/metricsview/executor/executor_validate.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -659,12 +659,12 @@ func (e *Executor) validateAndRewriteSchema(ctx context.Context, res *ValidateMe
659659
}
660660
types := make(map[string]*runtimev1.Type, len(schema.Fields))
661661
for _, f := range schema.Fields {
662-
types[f.Name] = f.Type
662+
types[strings.ToLower(f.Name)] = f.Type
663663
}
664664

665665
// Check that the measures are not strings
666666
for i, m := range e.metricsView.Measures {
667-
typ, ok := types[m.Name]
667+
typ, ok := types[strings.ToLower(m.Name)]
668668
if !ok {
669669
// Don't error: schemas are not always reliable
670670
continue
@@ -684,7 +684,7 @@ func (e *Executor) validateAndRewriteSchema(ctx context.Context, res *ValidateMe
684684
for _, d := range e.metricsView.Dimensions {
685685
// Find the dimension's data type, and infer the dimension type
686686
var code runtimev1.Type_Code
687-
typ, ok := types[d.Name]
687+
typ, ok := types[strings.ToLower(d.Name)]
688688
if ok {
689689
code = typ.Code
690690
d.DataType = typ
@@ -703,7 +703,7 @@ func (e *Executor) validateAndRewriteSchema(ctx context.Context, res *ValidateMe
703703
case runtimev1.Type_CODE_UNSPECIFIED, runtimev1.Type_CODE_DATE:
704704
// Unspecified types (e.g. if type detection failed) or DATE types only default to the TIME dimension type if they are the default time dimension.
705705
// Otherwise they default to CATEGORICAL.
706-
if d.Name == e.metricsView.TimeDimension {
706+
if strings.EqualFold(d.Name, e.metricsView.TimeDimension) {
707707
d.Type = runtimev1.MetricsViewSpec_DIMENSION_TYPE_TIME
708708
} else {
709709
d.Type = runtimev1.MetricsViewSpec_DIMENSION_TYPE_CATEGORICAL

‎runtime/reconcilers/metrics_view_test.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,98 @@ import (
99
"github.com/stretchr/testify/require"
1010
)
1111

12+
func TestMetricsViewTimeCaseInsensitive(t *testing.T) {
13+
rt, id := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{
14+
Files: map[string]string{
15+
"m1.sql": `SELECT '2024-01-01T00:00:00Z'::TIMESTAMP AS TiMe, 1 AS num`,
16+
"mv1.yaml": `
17+
type: metrics_view
18+
model: m1
19+
timeseries: TiMe
20+
measures:
21+
- name: num
22+
expression: sum(num)
23+
explore:
24+
skip: true
25+
`,
26+
"mv2.yaml": `
27+
type: metrics_view
28+
model: m1
29+
timeseries: TiMe
30+
dimensions:
31+
- column: TiMe
32+
measures:
33+
- name: num
34+
expression: sum(num)
35+
explore:
36+
skip: true
37+
`,
38+
"mv3.yaml": `
39+
type: metrics_view
40+
model: m1
41+
timeseries: time
42+
dimensions:
43+
- name: time
44+
column: TiMe
45+
measures:
46+
- name: num
47+
expression: sum(num)
48+
explore:
49+
skip: true
50+
`,
51+
"mv4.yaml": `
52+
type: metrics_view
53+
model: m1
54+
timeseries: time
55+
dimensions:
56+
- column: TiMe
57+
measures:
58+
- name: num
59+
expression: sum(num)
60+
explore:
61+
skip: true
62+
`,
63+
"mv5.yaml": `
64+
type: metrics_view
65+
model: m1
66+
timeseries: TiMe
67+
dimensions:
68+
- name: time
69+
column: TiMe
70+
measures:
71+
- name: num
72+
expression: sum(num)
73+
explore:
74+
skip: true
75+
`,
76+
},
77+
})
78+
testruntime.RequireReconcileState(t, rt, id, 5, 1, 2)
79+
80+
r := testruntime.GetResource(t, rt, id, runtime.ResourceKindMetricsView, "mv1")
81+
require.Empty(t, r.Meta.ReconcileError)
82+
d := r.GetMetricsView().State.ValidSpec.Dimensions[0]
83+
require.Equal(t, runtimev1.MetricsViewSpec_DIMENSION_TYPE_TIME, d.Type)
84+
require.Equal(t, runtimev1.Type_CODE_TIMESTAMP, d.DataType.Code)
85+
86+
r = testruntime.GetResource(t, rt, id, runtime.ResourceKindMetricsView, "mv2")
87+
require.Empty(t, r.Meta.ReconcileError)
88+
d = r.GetMetricsView().State.ValidSpec.Dimensions[0]
89+
require.Equal(t, runtimev1.MetricsViewSpec_DIMENSION_TYPE_TIME, d.Type)
90+
require.Equal(t, runtimev1.Type_CODE_TIMESTAMP, d.DataType.Code)
91+
92+
r = testruntime.GetResource(t, rt, id, runtime.ResourceKindMetricsView, "mv3")
93+
require.Empty(t, r.Meta.ReconcileError)
94+
d = r.GetMetricsView().State.ValidSpec.Dimensions[0]
95+
require.Equal(t, runtimev1.MetricsViewSpec_DIMENSION_TYPE_TIME, d.Type)
96+
require.Equal(t, runtimev1.Type_CODE_TIMESTAMP, d.DataType.Code)
97+
98+
testruntime.RequireParseErrors(t, rt, id, map[string]string{
99+
"/mv4.yaml": "does not match the case of time dimension",
100+
"/mv5.yaml": "does not match the case of time dimension",
101+
})
102+
}
103+
12104
func TestMetricsViewTimeTypes(t *testing.T) {
13105
rt, id := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{
14106
Files: map[string]string{

0 commit comments

Comments
 (0)