From 567915f5e39343bbbd6e8ebe33b79f589104dabf Mon Sep 17 00:00:00 2001 From: Jacek Migdal Date: Tue, 10 Jun 2025 12:58:58 +0200 Subject: [PATCH 1/4] Fix bug in correctness. --- platform/optimize/split_time_range_ext.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/platform/optimize/split_time_range_ext.go b/platform/optimize/split_time_range_ext.go index 238096842..a3ba2280f 100644 --- a/platform/optimize/split_time_range_ext.go +++ b/platform/optimize/split_time_range_ext.go @@ -218,6 +218,14 @@ func (s splitTimeRangeExt) transformQuery(query *model.Query, properties map[str ), ) + if subquery.WhereClause != nil { + whereClause = model.NewInfixExpr( + subquery.WhereClause, + "AND", + whereClause, + ) + } + subquery.WhereClause = whereClause subqueries = append(subqueries, subquery) } From 0b176871f2bd922c3a44baa386fd083580bce021 Mon Sep 17 00:00:00 2001 From: Jacek Migdal Date: Tue, 10 Jun 2025 13:20:27 +0200 Subject: [PATCH 2/4] Add failing test --- .../optimize/split_time_range_ext_test.go | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 platform/optimize/split_time_range_ext_test.go diff --git a/platform/optimize/split_time_range_ext_test.go b/platform/optimize/split_time_range_ext_test.go new file mode 100644 index 000000000..a1c8463eb --- /dev/null +++ b/platform/optimize/split_time_range_ext_test.go @@ -0,0 +1,35 @@ +// Copyright Quesma, licensed under the Elastic License 2.0. +// SPDX-License-Identifier: Elastic-2.0 +package optimize + +import ( + "github.com/QuesmaOrg/quesma/platform/model" + "github.com/k0kubun/pp" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestSplitTimeRange_no_change(t *testing.T) { + transformer := &splitTimeRangeExt{} + plan := model.NewExecutionPlan([]*model.Query{{ + SelectCommand: model.SelectCommand{ + Columns: []model.Expr{model.NewColumnRef("*")}, + FromClause: model.NewTableRef("foo"), + Limit: 500, + OrderBy: []model.OrderByExpr{model.NewOrderByExpr(model.NewColumnRef("@timestamp"), model.DescOrder)}, + WhereClause: model.And([]model.Expr{ + model.NewInfixExpr(model.NewColumnRef("@timestamp"), ">=", model.NewFunction("fromUnixTimestamp64Milli", model.NewLiteral(int64(1749496092480)))), + model.NewInfixExpr(model.NewColumnRef("@timestamp"), "<=", model.NewFunction("fromUnixTimestamp64Milli", model.NewLiteral(int64(1749550092480)))), + model.NewInfixExpr(model.NewColumnRef("status"), "=", model.NewLiteral("active")), + }), + }, + }}, nil) + + newPlan, err := transformer.Transform(plan, make(map[string]string)) + + assert.NoError(t, err) + + pp.Println("JM", newPlan.Queries) + assert.Equal(t, 1, len(newPlan.Queries)) + assert.Equal(t, plan.Queries[0].SelectCommand, newPlan.Queries[0].SelectCommand) +} From 5f1c42b67701c10756a0f6fec6362aece020a96a Mon Sep 17 00:00:00 2001 From: Jacek Migdal Date: Tue, 10 Jun 2025 13:36:01 +0200 Subject: [PATCH 3/4] Fix case in which we do not split at all --- platform/optimize/split_time_range_ext.go | 8 +++----- platform/optimize/split_time_range_ext_test.go | 4 +--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/platform/optimize/split_time_range_ext.go b/platform/optimize/split_time_range_ext.go index a3ba2280f..ac1b7ef8b 100644 --- a/platform/optimize/split_time_range_ext.go +++ b/platform/optimize/split_time_range_ext.go @@ -161,7 +161,7 @@ func (s splitTimeRangeExt) getSplitPoints(foundTimeRange timeRange, properties m } } - result := []timeRangeLimit{foundTimeRange.lowerLimit} + result := []timeRangeLimit{foundTimeRange.lowerLimit, foundTimeRange.upperLimit} for _, shorterTimeRangeMinute := range shorterTimeRangesMinutes { var splitPoint timeRangeLimit @@ -170,13 +170,11 @@ func (s splitTimeRangeExt) getSplitPoints(foundTimeRange timeRange, properties m } else { splitPoint = timeRangeLimit{value: foundTimeRange.upperLimit.value - shorterTimeRangeMinute*int64(time.Minute.Seconds()), funcName: foundTimeRange.upperLimit.funcName} } - if splitPoint.value >= foundTimeRange.lowerLimit.value && splitPoint.value <= foundTimeRange.upperLimit.value { + if splitPoint.value > foundTimeRange.lowerLimit.value && splitPoint.value < foundTimeRange.upperLimit.value { result = append(result, splitPoint) } } - result = append(result, foundTimeRange.upperLimit) - sort.Slice(result, func(i, j int) bool { return result[i].value >= result[j].value }) @@ -198,7 +196,7 @@ func (s splitTimeRangeExt) transformQuery(query *model.Query, properties map[str queries = append(queries, query) return queries, nil } - + for i := 1; i < len(splitPoints); i++ { subquery := query.SelectCommand diff --git a/platform/optimize/split_time_range_ext_test.go b/platform/optimize/split_time_range_ext_test.go index a1c8463eb..09edab13b 100644 --- a/platform/optimize/split_time_range_ext_test.go +++ b/platform/optimize/split_time_range_ext_test.go @@ -4,7 +4,6 @@ package optimize import ( "github.com/QuesmaOrg/quesma/platform/model" - "github.com/k0kubun/pp" "github.com/stretchr/testify/assert" "testing" ) @@ -18,7 +17,7 @@ func TestSplitTimeRange_no_change(t *testing.T) { Limit: 500, OrderBy: []model.OrderByExpr{model.NewOrderByExpr(model.NewColumnRef("@timestamp"), model.DescOrder)}, WhereClause: model.And([]model.Expr{ - model.NewInfixExpr(model.NewColumnRef("@timestamp"), ">=", model.NewFunction("fromUnixTimestamp64Milli", model.NewLiteral(int64(1749496092480)))), + model.NewInfixExpr(model.NewColumnRef("@timestamp"), ">=", model.NewFunction("fromUnixTimestamp64Milli", model.NewLiteral(int64(1749549192480)))), model.NewInfixExpr(model.NewColumnRef("@timestamp"), "<=", model.NewFunction("fromUnixTimestamp64Milli", model.NewLiteral(int64(1749550092480)))), model.NewInfixExpr(model.NewColumnRef("status"), "=", model.NewLiteral("active")), }), @@ -29,7 +28,6 @@ func TestSplitTimeRange_no_change(t *testing.T) { assert.NoError(t, err) - pp.Println("JM", newPlan.Queries) assert.Equal(t, 1, len(newPlan.Queries)) assert.Equal(t, plan.Queries[0].SelectCommand, newPlan.Queries[0].SelectCommand) } From 6397b54c3f31869fdb0428e2457c555e2362e9f3 Mon Sep 17 00:00:00 2001 From: Jacek Migdal Date: Tue, 10 Jun 2025 14:20:29 +0200 Subject: [PATCH 4/4] Merge failed test --- platform/optimize/split_time_range_ext.go | 4 +- .../optimize/split_time_range_ext_test.go | 75 ++++++++++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/platform/optimize/split_time_range_ext.go b/platform/optimize/split_time_range_ext.go index ac1b7ef8b..20ec1602b 100644 --- a/platform/optimize/split_time_range_ext.go +++ b/platform/optimize/split_time_range_ext.go @@ -196,7 +196,7 @@ func (s splitTimeRangeExt) transformQuery(query *model.Query, properties map[str queries = append(queries, query) return queries, nil } - + for i := 1; i < len(splitPoints); i++ { subquery := query.SelectCommand @@ -212,7 +212,7 @@ func (s splitTimeRangeExt) transformQuery(query *model.Query, properties map[str ), "AND", model.NewInfixExpr( - model.NewColumnRef(foundTimeRange.columnName), "<", endExpr, + model.NewColumnRef(foundTimeRange.columnName), "<=", endExpr, ), ) diff --git a/platform/optimize/split_time_range_ext_test.go b/platform/optimize/split_time_range_ext_test.go index 09edab13b..53e3e1694 100644 --- a/platform/optimize/split_time_range_ext_test.go +++ b/platform/optimize/split_time_range_ext_test.go @@ -4,11 +4,12 @@ package optimize import ( "github.com/QuesmaOrg/quesma/platform/model" + "github.com/k0kubun/pp" "github.com/stretchr/testify/assert" "testing" ) -func TestSplitTimeRange_no_change(t *testing.T) { +func TestSplitTimeRange_no_change_15m(t *testing.T) { transformer := &splitTimeRangeExt{} plan := model.NewExecutionPlan([]*model.Query{{ SelectCommand: model.SelectCommand{ @@ -31,3 +32,75 @@ func TestSplitTimeRange_no_change(t *testing.T) { assert.Equal(t, 1, len(newPlan.Queries)) assert.Equal(t, plan.Queries[0].SelectCommand, newPlan.Queries[0].SelectCommand) } + +func TestSplitTimeRange_split_1h(t *testing.T) { + transformer := &splitTimeRangeExt{} + timestamp := 1749550092480 + timestamp15MinutesAgo := timestamp - (15 * 60 * 1000) // 15 minutes in milliseconds + timestampHourAgo := timestamp - (60 * 60 * 1000) // 1 hour in milliseconds + + newTimestampWhereExpr := func(operator string, timestampArg int64) model.Expr { + return model.NewInfixExpr( + model.NewColumnRef("@timestamp"), + operator, + model.NewFunction("fromUnixTimestamp64Milli", model.NewLiteral(timestampArg)), + ) + } + + plan := model.NewExecutionPlan([]*model.Query{{ + SelectCommand: model.SelectCommand{ + Columns: []model.Expr{model.NewColumnRef("*")}, + FromClause: model.NewTableRef("foo"), + Limit: 500, + OrderBy: []model.OrderByExpr{model.NewOrderByExpr(model.NewColumnRef("@timestamp"), model.DescOrder)}, + WhereClause: model.And([]model.Expr{ + newTimestampWhereExpr(">=", int64(timestampHourAgo)), + newTimestampWhereExpr("<=", int64(timestamp)), + model.NewInfixExpr(model.NewColumnRef("status"), "=", model.NewLiteral("active")), + }), + }, + }}, nil) + + newPlan, err := transformer.Transform(plan, make(map[string]string)) + + assert.NoError(t, err) + + assert.Equal(t, 2, len(newPlan.Queries)) + for q := range newPlan.Queries { + assert.Equal(t, plan.Queries[0].SelectCommand.Columns, newPlan.Queries[q].SelectCommand.Columns) + assert.Equal(t, plan.Queries[0].SelectCommand.FromClause, newPlan.Queries[q].SelectCommand.FromClause) + assert.Equal(t, plan.Queries[0].SelectCommand.Limit, newPlan.Queries[q].SelectCommand.Limit) + assert.Equal(t, plan.Queries[0].SelectCommand.OrderBy, newPlan.Queries[q].SelectCommand.OrderBy) + } + + expectedWhereA := model.And([]model.Expr{ + model.And([]model.Expr{ + newTimestampWhereExpr(">=", int64(timestampHourAgo)), + newTimestampWhereExpr("<=", int64(timestamp)), + model.NewInfixExpr(model.NewColumnRef("status"), "=", model.NewLiteral("active")), + }), + model.And([]model.Expr{ + newTimestampWhereExpr(">=", int64(timestampHourAgo)), + newTimestampWhereExpr("<=", int64(timestamp15MinutesAgo)), + }), + }) + expectedWhereB := model.And([]model.Expr{ + model.And([]model.Expr{ + newTimestampWhereExpr(">=", int64(timestampHourAgo)), + newTimestampWhereExpr("<=", int64(timestamp)), + model.NewInfixExpr(model.NewColumnRef("status"), "=", model.NewLiteral("active")), + }), + model.And([]model.Expr{ + newTimestampWhereExpr(">=", int64(timestamp15MinutesAgo)), + newTimestampWhereExpr("<=", int64(timestamp)), + }), + }) + + pp.Println("Expected Where A: ", model.AsString(expectedWhereA)) + pp.Println("Actual Where A: ", model.AsString(newPlan.Queries[0].SelectCommand.WhereClause)) + pp.Println("Expected Where B: ", model.AsString(expectedWhereB)) + pp.Println("Actual Where B: ", model.AsString(newPlan.Queries[1].SelectCommand.WhereClause)) + + assert.Equal(t, expectedWhereA, newPlan.Queries[0].SelectCommand.WhereClause) + assert.Equal(t, expectedWhereB, newPlan.Queries[1].SelectCommand.WhereClause) +}