Skip to content

Commit e9f274f

Browse files
committed
handle nulls during cast for like op for ch (#8846)
* handle nulls during cast for like op for ch * test * comment * comment
1 parent a9575f8 commit e9f274f

File tree

5 files changed

+80
-15
lines changed

5 files changed

+80
-15
lines changed

runtime/drivers/olap.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,9 +304,12 @@ func (d Dialect) SupportsILike() bool {
304304
return d != DialectDruid && d != DialectPinot && d != DialectStarRocks
305305
}
306306

307-
// RequiresCastForLike returns true if the dialect requires an expression used in a LIKE or ILIKE condition to explicitly be cast to type TEXT.
308-
func (d Dialect) RequiresCastForLike() bool {
309-
return d == DialectClickHouse
307+
// GetCastExprForLike returns the cast expression for use in a LIKE or ILIKE condition, or an empty string if no cast is necessary.
308+
func (d Dialect) GetCastExprForLike() string {
309+
if d == DialectClickHouse {
310+
return "::Nullable(TEXT)"
311+
}
312+
return ""
310313
}
311314

312315
func (d Dialect) SupportsRegexMatch() bool {

runtime/metricsview/astexpr.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,7 @@ func (b *sqlExprBuilder) writeILikeCondition(left, right *Expression, leftOverri
405405
}
406406
}
407407

408-
if b.ast.Dialect.RequiresCastForLike() {
409-
b.writeString("::TEXT")
410-
}
408+
b.writeString(b.ast.Dialect.GetCastExprForLike())
411409

412410
if not {
413411
b.writeString(" NOT ILIKE ")
@@ -420,9 +418,7 @@ func (b *sqlExprBuilder) writeILikeCondition(left, right *Expression, leftOverri
420418
return err
421419
}
422420

423-
if b.ast.Dialect.RequiresCastForLike() {
424-
b.writeString("::TEXT")
425-
}
421+
b.writeString(b.ast.Dialect.GetCastExprForLike())
426422
} else if b.ast.Dialect.SupportsRegexMatch() {
427423
if not {
428424
b.writeString(" NOT ")
@@ -475,9 +471,7 @@ func (b *sqlExprBuilder) writeILikeCondition(left, right *Expression, leftOverri
475471
return err
476472
}
477473
}
478-
if b.ast.Dialect.RequiresCastForLike() {
479-
b.writeString("::TEXT")
480-
}
474+
b.writeString(b.ast.Dialect.GetCastExprForLike())
481475
b.writeByte(')')
482476

483477
if not {
@@ -491,9 +485,7 @@ func (b *sqlExprBuilder) writeILikeCondition(left, right *Expression, leftOverri
491485
if err != nil {
492486
return err
493487
}
494-
if b.ast.Dialect.RequiresCastForLike() {
495-
b.writeString("::TEXT")
496-
}
488+
b.writeString(b.ast.Dialect.GetCastExprForLike())
497489
b.writeByte(')')
498490
}
499491

runtime/queries/metricsview_aggregation_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ func TestMetricViewAggregationAgainstClickHouse(t *testing.T) {
6363
t.Run("testMetricsViewsAggregation_comparison_with_offset_and_limit_and_delta", func(t *testing.T) {
6464
testMetricsViewsAggregation_comparison_with_offset_and_limit_and_delta(t, rt, instanceID)
6565
})
66+
t.Run("testMetricsViewsAggregation_like_nullable", func(t *testing.T) { testMetricsViewsAggregation_like_nullable(t, rt, instanceID) })
6667
}
6768

6869
func TestMetricViewAggregationAgainstStarRocks(t *testing.T) {
@@ -4959,6 +4960,61 @@ func testMetricsViewAggregation_percent_of_totals_with_limit(t *testing.T, rt *r
49594960
require.Equal(t, "news.google.com,256.00,0.23", fieldsToString2digits(rows[i], "domain", "total_records", "total_records__pt"))
49604961
}
49614962

4963+
func testMetricsViewsAggregation_like_nullable(t *testing.T, rt *runtime.Runtime, instanceID string) {
4964+
// Test LIKE on a Nullable(String) column
4965+
q := &queries.MetricsViewAggregation{
4966+
MetricsViewName: "ad_bids_nullable_metrics",
4967+
Dimensions: []*runtimev1.MetricsViewAggregationDimension{
4968+
{
4969+
Name: "publisher",
4970+
},
4971+
},
4972+
Measures: []*runtimev1.MetricsViewAggregationMeasure{
4973+
{
4974+
Name: "total_records",
4975+
},
4976+
},
4977+
Where: expressionpb.Like(
4978+
expressionpb.Identifier("publisher"),
4979+
expressionpb.Value(structpb.NewStringValue("%oo%")),
4980+
),
4981+
Sort: []*runtimev1.MetricsViewAggregationSort{
4982+
{
4983+
Name: "publisher",
4984+
},
4985+
},
4986+
SecurityClaims: testClaims(),
4987+
}
4988+
err := q.Resolve(context.Background(), rt, instanceID, 0)
4989+
require.NoError(t, err)
4990+
require.NotEmpty(t, q.Result)
4991+
4992+
rows := q.Result.Data
4993+
require.Len(t, rows, 3)
4994+
require.Equal(t, "Facebook,19341", fieldsToString(rows[0], "publisher", "total_records"))
4995+
require.Equal(t, "Google,18763", fieldsToString(rows[1], "publisher", "total_records"))
4996+
require.Equal(t, "Yahoo,18593", fieldsToString(rows[2], "publisher", "total_records"))
4997+
4998+
// Test NOT LIKE with nullable column — should include NULL publisher rows
4999+
q.Where = expressionpb.NotLike(
5000+
expressionpb.Identifier("publisher"),
5001+
expressionpb.Value(structpb.NewStringValue("%oo%")),
5002+
)
5003+
err = q.Resolve(context.Background(), rt, instanceID, 0)
5004+
require.NoError(t, err)
5005+
require.NotEmpty(t, q.Result)
5006+
5007+
rows = q.Result.Data
5008+
require.GreaterOrEqual(t, len(rows), 2)
5009+
// Should contain Microsoft and null publishers
5010+
publishers := make(map[string]bool)
5011+
for _, row := range rows {
5012+
publishers[fieldsToString(row, "publisher")] = true
5013+
}
5014+
require.True(t, publishers["Microsoft"])
5015+
require.True(t, publishers["null"])
5016+
}
5017+
49625018
func fieldsToString2digits(row *structpb.Struct, args ...string) string {
49635019
s := make([]string, 0, len(args))
49645020
for _, arg := range args {
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
type: metrics_view
2+
model: ad_bids_nullable
3+
timeseries: timestamp
4+
dimensions:
5+
- column: publisher
6+
- column: domain
7+
measures:
8+
- name: total_records
9+
expression: COUNT(*)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
SELECT
2+
toNullable(publisher) AS publisher,
3+
toNullable(domain) AS domain,
4+
timestamp
5+
FROM {{ ref "ad_bids_source" }}

0 commit comments

Comments
 (0)