Skip to content

Commit

Permalink
Quickfix: bool column with terms aggr (#1145)
Browse files Browse the repository at this point in the history 
Response for a `bool` bucket needs to be slightly different.

Before:
<img width="1728" alt="Screenshot 2024-12-28 at 15 42 21"
src="https://github.com/user-attachments/assets/adf4c2e9-067e-4964-87fb-e2ccaead19b9"
/>
After:
<img width="1728" alt="Screenshot 2024-12-28 at 15 17 41"
src="https://github.com/user-attachments/assets/3713f95e-53b2-46a6-a383-d1fd665ce02e"
/>
  • Loading branch information
@trzysiek
trzysiek authored Dec 28, 2024
1 parent 9173255 commit d4745d3
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 28 deletions.
14 changes: 13 additions & 1 deletion quesma/model/bucket_aggregations/terms.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,25 @@ func (query Terms) TranslateSqlResponseToJson(rows []model.QueryResultRow) model
for _, row := range rows {
docCount := query.docCount(row)
bucket := model.JsonMap{
"key": query.key(row),
"doc_count": docCount,
}
if query.significant {
bucket["score"] = docCount
bucket["bg_count"] = docCount
}

// response for bool keys is different
key := query.key(row)
if boolPtr, isBoolPtr := key.(*bool); isBoolPtr {
key = *boolPtr
}
if keyAsBool, ok := key.(bool); ok {
bucket["key"] = util.BoolToInt(keyAsBool)
bucket["key_as_string"] = util.BoolToString(keyAsBool)
} else {
bucket["key"] = key
}

response = append(response, bucket)
}

Expand Down
7 changes: 3 additions & 4 deletions quesma/model/pipeline_aggregations/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ func calculateResultWhenMissingCommonForDiffAggregations(ctx context.Context, pa
if row.LastColValue() != nil {
firstNonNilIndex = i + rowsWithNilValueCnt
break
} else {
resultRow := row.Copy()
resultRow.Cols[len(resultRow.Cols)-1].Value = nil
resultRows = append(resultRows, resultRow)
}
resultRow := row.Copy()
resultRow.Cols[len(resultRow.Cols)-1].Value = nil
resultRows = append(resultRows, resultRow)
}
if firstNonNilIndex == -1 {
return resultRows
Expand Down
8 changes: 1 addition & 7 deletions quesma/model/pipeline_aggregations/max_bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,7 @@ func (query MaxBucket) calculateSingleMaxBucket(parentRows []model.QueryResultRo
var resultValue any
var resultKeys []any

firstNonNilIndex := -1
for i, row := range parentRows {
if row.LastColValue() != nil {
firstNonNilIndex = i
break
}
}
firstNonNilIndex := model.FirstNonNilIndex(parentRows)
if firstNonNilIndex == -1 {
resultRow := parentRows[0].Copy()
resultRow.Cols[len(resultRow.Cols)-1].Value = model.JsonMap{
Expand Down
8 changes: 1 addition & 7 deletions quesma/model/pipeline_aggregations/sum_bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,7 @@ func (query SumBucket) CalculateResultWhenMissing(parentRows []model.QueryResult
func (query SumBucket) calculateSingleSumBucket(parentRows []model.QueryResultRow) model.QueryResultRow {
var resultValue any

firstNonNilIndex := -1
for i, row := range parentRows {
if row.LastColValue() != nil {
firstNonNilIndex = i
break
}
}
firstNonNilIndex := model.FirstNonNilIndex(parentRows)
if firstNonNilIndex == -1 {
resultRow := parentRows[0].Copy()
resultRow.Cols[len(resultRow.Cols)-1].Value = model.JsonMap{
Expand Down
9 changes: 9 additions & 0 deletions quesma/model/query_result.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,12 @@ func (r *QueryResultRow) Copy() QueryResultRow {
func (r *QueryResultRow) LastColValue() any {
return r.Cols[len(r.Cols)-1].Value
}

func FirstNonNilIndex(rows []QueryResultRow) int {
for i, row := range rows {
if row.LastColValue() != nil {
return i
}
}
return -1
}
64 changes: 64 additions & 0 deletions quesma/testdata/aggregation_requests_2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"time"
)

var randomTrueVariableUsedBelow = true

// Goland lags a lot when you edit aggregation_requests.go file, so let's add new tests to this one.

var AggregationTests2 = []AggregationTestCase{
Expand Down Expand Up @@ -5290,6 +5292,68 @@ var AggregationTests2 = []AggregationTestCase{
LIMIT 3`},
},
{ // [77]
TestName: "terms with bool field",
QueryRequestJson: `
{
"aggs": {
"terms": {
"terms": {
"field": "Cancelled",
"size": 2
}
}
},
"size": 0,
"track_total_hits": true
}`,
// I omit "took", "timed_out", "_shards", and "hits" from the response for brevity (they can also be easily unit-tested)
ExpectedResponse: `
{
"aggregations": {
"terms": {
"doc_count_error_upper_bound": 0,
"sum_other_doc_count": 38654,
"buckets": [
{
"key": 0,
"key_as_string": "false",
"doc_count": 11344
},
{
"key": 1,
"key_as_string": "true",
"doc_count": 2
}
]
}
}
}`,
ExpectedPancakeResults: []model.QueryResultRow{
{Cols: []model.QueryResultCol{
model.NewQueryResultCol("aggr__terms__parent_count", int64(50000)),
model.NewQueryResultCol("aggr__terms__key_0", nil),
model.NewQueryResultCol("aggr__terms__count", int64(12000)),
}},
{Cols: []model.QueryResultCol{
model.NewQueryResultCol("aggr__terms__parent_count", int64(50000)),
model.NewQueryResultCol("aggr__terms__key_0", false),
model.NewQueryResultCol("aggr__terms__count", int64(11344)),
}},
{Cols: []model.QueryResultCol{
model.NewQueryResultCol("aggr__terms__parent_count", int64(50000)),
model.NewQueryResultCol("aggr__terms__key_0", &randomTrueVariableUsedBelow), // used here
model.NewQueryResultCol("aggr__terms__count", int64(2)),
}},
},
ExpectedPancakeSQL: `
SELECT sum(count(*)) OVER () AS "aggr__terms__parent_count",
"Cancelled" AS "aggr__terms__key_0", count(*) AS "aggr__terms__count"
FROM __quesma_table_name
GROUP BY "Cancelled" AS "aggr__terms__key_0"
ORDER BY "aggr__terms__count" DESC, "aggr__terms__key_0" ASC
LIMIT 3`,
},
{ // [78]
TestName: `Escaping of ', \, \n, and \t in some example aggregations. No tests for other escape characters, e.g. \r or 'b. Add if needed.`,
QueryRequestJson: `
{
Expand Down
6 changes: 4 additions & 2 deletions quesma/testdata/kibana-visualize/aggregation_requests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,8 @@ var AggregationTests = []testdata.AggregationTestCase{
"95.0": 15480.335426897316
}
},
"key": false,
"key": 0,
"key_as_string": "false",
"doc_count": 2974
},
{
Expand Down Expand Up @@ -991,7 +992,8 @@ var AggregationTests = []testdata.AggregationTestCase{
"95.0": 14463.254101562497
}
},
"key": true,
"key": 1,
"key_as_string": "true",
"doc_count": 419
}
]
Expand Down
16 changes: 9 additions & 7 deletions quesma/testdata/opensearch-visualize/pipeline_aggregation_requests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ var PipelineAggregationTests = []testdata.AggregationTestCase{
"key": 1714871400000,
"key_as_string": "2024-05-05T01:10:00.000"
},
{
{
"1": {
"value": 0.0
},
Expand All @@ -999,7 +999,7 @@ var PipelineAggregationTests = []testdata.AggregationTestCase{
"key": 1714872000000,
"key_as_string": "2024-05-05T01:20:00.000"
},
{
{
"1": {
"value": 0.0
},
Expand All @@ -1010,7 +1010,7 @@ var PipelineAggregationTests = []testdata.AggregationTestCase{
"key": 1714872600000,
"key_as_string": "2024-05-05T01:30:00.000"
},
{
{
"1": {
"value": 0.0
},
Expand All @@ -1021,7 +1021,7 @@ var PipelineAggregationTests = []testdata.AggregationTestCase{
"key": 1714873200000,
"key_as_string": "2024-05-05T01:40:00.000"
},
{
{
"1": {
"value": 0.0
},
Expand All @@ -1032,7 +1032,7 @@ var PipelineAggregationTests = []testdata.AggregationTestCase{
"key": 1714873800000,
"key_as_string": "2024-05-05T01:50:00.000"
},
{
{
"1": {
"value": 0.0
},
Expand Down Expand Up @@ -3345,11 +3345,13 @@ var PipelineAggregationTests = []testdata.AggregationTestCase{
"buckets": [
{
"doc_count": 260,
"key": true
"key": 1,
"key_as_string": "true"
},
{
"doc_count": 1923,
"key": false
"key": 0,
"key_as_string": "false"
}
],
"doc_count_error_upper_bound": 0,
Expand Down
14 changes: 14 additions & 0 deletions quesma/util/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,20 @@ func ExtractNumeric64(value any) float64 {
return asFloat64
}

func BoolToInt(b bool) int {
if b {
return 1
}
return 0
}

func BoolToString(b bool) string {
if b {
return "true"
}
return "false"
}

// SingleQuote is a simple helper function: str -> 'str'
func SingleQuote(value string) string {
return "'" + value + "'"
Expand Down

0 comments on commit d4745d3

Please sign in to comment.