Cleanup 2

Author: trzysiek

quesma/queryparser/pancake_sql_query_generation.go

@@ -310,24 +310,12 @@ func (p *pancakeSqlQueryGenerator) generateSelectCommand(aggregation *pancakeMod
 		if layer.nextBucketAggregation != nil {
 			if combinator, isCombinator := layer.nextBucketAggregation.queryType.(bucket_aggregations.CombinatorAggregationInterface); isCombinator {
 				var isFilter bool
-				//pp.Println(combinator)
 				switch combinator.(type) {
 				case *bucket_aggregations.FilterAgg, bucket_aggregations.Filters:
 					isFilter = true
 				}
-				fmt.Println("isFilter: ", isFilter, len(aggregation.layers))
-				//pp.Println(aggregation.layers[0])
-				if isFilter && i == 0 && len(aggregation.layers) > 1 && len(layer.currentMetricAggregations) == 0 && len(layer.currentPipelineAggregations) == 0 {
-					// If filter is in the first layer, we can just add it to the where clause
-					switch combinatorTyped := combinator.(type) {
-					case bucket_aggregations.FilterAgg:
-						//aggregation.whereClause = model.And([]model.Expr{aggregation.whereClause, combinatorTyped.WhereClause})
-					case bucket_aggregations.Filters:
-						// TODO accept second
-						fmt.Println("Adding ", combinatorTyped.Filters[0].Sql.WhereClause)
-						aggregation.whereClause = model.And([]model.Expr{aggregation.whereClause, combinatorTyped.Filters[0].Sql.WhereClause}) // TODO check [0]
-					}
-				} else {
+				filterAlreadyInWhereClause := i == 0 && len(aggregation.layers) > 1 && len(layer.currentMetricAggregations) == 0 && len(layer.currentPipelineAggregations) == 0
+				if !isFilter || !filterAlreadyInWhereClause {
 					addIfCombinators = append(addIfCombinators, addIfCombinator{len(selectColumns), combinator})
 				}
 			}
@@ -353,7 +341,6 @@ func (p *pancakeSqlQueryGenerator) generateSelectCommand(aggregation *pancakeMod
 	// this change selects by adding -If suffix, e.g. count(*) -> countIf(response_time < 1000)
 	// they may also add more columns with different prefix and where clauses
 	var combinatorWhere []model.Expr
-	fmt.Println("len(addIfCombinators): ", len(addIfCombinators))
 	for i := len(addIfCombinators) - 1; i >= 0; i-- { // reverse order is important
 		combinator := addIfCombinators[i]
 		selectsBefore := selectColumns[:combinator.selectNr]

quesma/queryparser/pancake_sql_query_generation_test.go

@@ -165,7 +165,7 @@ func TestPancakeQueryGeneration(t *testing.T) {
 			if len(expectedMinusActual) != 0 {
 				pp.Println("EXPECTED diff", expectedMinusActual)
 			}
-			pp.Println("ACTUAL", pancakeJson)
+			//pp.Println("ACTUAL", pancakeJson)
 			//pp.Println("EXPECTED", expectedAggregationsPart)
 			assert.True(t, util.AlmostEmpty(actualMinusExpected, acceptableDifference))
 			assert.True(t, util.AlmostEmpty(expectedMinusActual, acceptableDifference))

quesma/queryparser/pancake_transformer.go

@@ -403,18 +403,26 @@ func (a *pancakeTransformer) aggregationTreeToPancakes(topLevel pancakeAggregati
 			whereClause: topLevel.whereClause,
 			sampleLimit: sampleLimit,
 		}
+		pancakeResults = append(pancakeResults, &newPancake)
+
+		// TODO: if both top_hits/top_metrics, and filters, it probably won't work...
+		// Care: order of these two functions is unfortunately important.
+		// Should be fixed after this TODO
+		newFiltersPancakes := a.createFiltersPancakes(&newPancake)
 		additionalTopHitPancakes, err := a.createTopHitAndTopMetricsPancakes(&newPancake)
 		if err != nil {
 			return nil, err
 		}
-		pancakeResults = append(pancakeResults, &newPancake)
+
 		pancakeResults = append(pancakeResults, additionalTopHitPancakes...)
-		pancakeResults = append(pancakeResults, a.createFiltersPancakes(&newPancake)...)
+		pancakeResults = append(pancakeResults, newFiltersPancakes...)
 	}
 
 	return
 }
 
+// createFiltersPancakes only does something, if first layer aggregation is Filters.
+// It creates new pancakes for each filter in that aggregation, and updates `pancake` to have only first filter.
 func (a *pancakeTransformer) createFiltersPancakes(pancake *pancakeModel) (newPancakes []*pancakeModel) {
 	if len(pancake.layers) == 0 || pancake.layers[0].nextBucketAggregation == nil {
 		return
@@ -423,9 +431,9 @@ func (a *pancakeTransformer) createFiltersPancakes(pancake *pancakeModel) (newPa
 	firstLayer := pancake.layers[0]
 	filters, isFilters := firstLayer.nextBucketAggregation.queryType.(bucket_aggregations.Filters)
 	canSimplyAddFilterToWhereClause := len(firstLayer.currentMetricAggregations) == 0 && len(firstLayer.currentPipelineAggregations) == 0
-	isItNeeded := len(pancake.layers) > 1
+	areNewPancakesReallyNeeded := len(pancake.layers) > 1 // if there is only one layer, it's better to get it done with combinators.
 
-	if !isFilters || !canSimplyAddFilterToWhereClause || !isItNeeded {
+	if !isFilters || !canSimplyAddFilterToWhereClause || !areNewPancakesReallyNeeded || len(filters.Filters) == 0 {
 		return
 	}
 
@@ -441,6 +449,7 @@ func (a *pancakeTransformer) createFiltersPancakes(pancake *pancakeModel) (newPa
 
 	// Then update original to have 1 filter as well
 	pancake.layers[0].nextBucketAggregation.queryType = filters.NewFiltersSingleFilter(0)
+	pancake.whereClause = model.And([]model.Expr{pancake.whereClause, filters.Filters[0].Sql.WhereClause})
 
 	return
 }

quesma/testdata/aggregation_requests.go

@@ -3487,9 +3487,9 @@ var AggregationTests = []AggregationTestCase{
 			  sumOrNull("taxful_total_price") AS "metric__time_offset_split__0__1_col_0",
 			  sumOrNull("taxful_total_price") AS "metric__time_offset_split__0__2_col_0"
 			FROM __quesma_table_name
-			WHERE ((("order_date">=fromUnixTimestamp64Milli(1708639056376) AND
-			  "order_date"<=fromUnixTimestamp64Milli(1709243856376)) OR
-			  ("order_date">=fromUnixTimestamp64Milli(1708034256376) AND "order_date"<=
+			WHERE ((("order_date">=fromUnixTimestamp64Milli(1708639056376) AND "order_date"
+			  <=fromUnixTimestamp64Milli(1709243856376)) OR ("order_date">=
+			  fromUnixTimestamp64Milli(1708034256376) AND "order_date"<=
 			  fromUnixTimestamp64Milli(1708639056376))) AND ("order_date">=
 			  fromUnixTimestamp64Milli(1708639056376) AND "order_date"<=
 			  fromUnixTimestamp64Milli(1709243856376)))
@@ -3504,14 +3504,12 @@ var AggregationTests = []AggregationTestCase{
 			  sumOrNull("taxful_total_price") AS "metric__time_offset_split__0__1_col_0",
 			  sumOrNull("taxful_total_price") AS "metric__time_offset_split__0__2_col_0"
 			FROM __quesma_table_name
-			WHERE (((("order_date">=fromUnixTimestamp64Milli(1708639056376) AND "order_date"
-			  <=fromUnixTimestamp64Milli(1709243856376)) OR ("order_date">=
-			  fromUnixTimestamp64Milli(1708034256376) AND "order_date"<=
-			  fromUnixTimestamp64Milli(1708639056376))) AND ("order_date">=
-			  fromUnixTimestamp64Milli(1708034256376) AND "order_date"<=
-			  fromUnixTimestamp64Milli(1708639056376))) AND ("order_date">=
-			  fromUnixTimestamp64Milli(1708034256376) AND "order_date"<=
-			  fromUnixTimestamp64Milli(1708639056376)))
+			WHERE ((("order_date">=fromUnixTimestamp64Milli(1708639056376) AND
+			  "order_date"<=fromUnixTimestamp64Milli(1709243856376)) OR
+			  ("order_date">=fromUnixTimestamp64Milli(1708034256376) AND
+			  "order_date"<=fromUnixTimestamp64Milli(1708639056376))) AND
+			  ("order_date">=fromUnixTimestamp64Milli(1708034256376) AND
+			  "order_date"<=fromUnixTimestamp64Milli(1708639056376)))
 			GROUP BY toInt64(toUnixTimestamp64Milli("order_date") / 86400000) AS
 			  "aggr__time_offset_split__0__key_0"
 			ORDER BY "aggr__time_offset_split__0__key_0" ASC`,