From 340a5509a740ba6959374a3599c7c1cadb2eb1d2 Mon Sep 17 00:00:00 2001
From: Jacek Migdal <jacek@quesma.com>
Date: Thu, 11 Jul 2024 17:35:32 +0200
Subject: [PATCH] Minor refactor query translator (#517)

I aim to unify json response generation. Currently, logic is split into
many places. I started with simplifying logic building aggregation
response tree.
---
 quesma/queryparser/query_translator.go | 42 ++++++++++----------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/quesma/queryparser/query_translator.go b/quesma/queryparser/query_translator.go
index f475de10d..41467166e 100644
--- a/quesma/queryparser/query_translator.go
+++ b/quesma/queryparser/query_translator.go
@@ -130,13 +130,21 @@ func (cw *ClickhouseQueryTranslator) makeResponseAggregationRecursive(query *mod
 	// fmt.Println("level1 :/", level1, " level2 B):", level2)
 
 	// or we need to go deeper
-	qp := queryprocessor.NewQueryProcessor(cw.Ctx)
-	var bucketsReturnMap []model.JsonMap
-	if query.Aggregators[aggregatorsLevel].SplitOverHowManyFields == 0 {
-		bucketsReturnMap = append(bucketsReturnMap, cw.makeResponseAggregationRecursive(query, ResultSet, aggregatorsLevel+1, selectLevel)...)
+	subResult := make(model.JsonMap, 1)
+	currentAggregator := query.Aggregators[aggregatorsLevel]
+	if currentAggregator.SplitOverHowManyFields == 0 {
+		subSubResult := cw.makeResponseAggregationRecursive(query, ResultSet, aggregatorsLevel+1, selectLevel)[0]
+		// Keyed and Filters aggregations are special and need to be wrapped in "buckets"
+		if currentAggregator.Keyed || currentAggregator.Filters {
+			subResult["buckets"] = subSubResult
+		} else {
+			subResult = subSubResult
+		}
 	} else {
+		var bucketsReturnMap []model.JsonMap
 		// normally it's just 1. It used to be just 1 before multi_terms aggregation, where we usually split over > 1 field
-		weSplitOverHowManyFields := query.Aggregators[aggregatorsLevel].SplitOverHowManyFields
+		qp := queryprocessor.NewQueryProcessor(cw.Ctx)
+		weSplitOverHowManyFields := currentAggregator.SplitOverHowManyFields
 		buckets := qp.SplitResultSetIntoBuckets(ResultSet, selectLevel+weSplitOverHowManyFields)
 		for _, bucket := range buckets {
 			newBuckets := cw.makeResponseAggregationRecursive(query, bucket, aggregatorsLevel+1, selectLevel+weSplitOverHowManyFields)
@@ -147,32 +155,14 @@ func (cw *ClickhouseQueryTranslator) makeResponseAggregationRecursive(query *mod
 			}
 			bucketsReturnMap = append(bucketsReturnMap, newBuckets...)
 		}
-	}
-
-	result := make(model.JsonMap, 1)
-	subResult := make(model.JsonMap, 1)
-
-	// The if below: very hacky, but works for now. I have an idea how to fix this and make code nice, but it'll take a while to refactor.
-	// Basically, for now every not-ending subaggregation has "buckets" key. Only exception is "sampler", which doesn't, thus this if.
-	//
-	// I'd like to keep an actual tree after the refactor, not a list of paths from root to leaf, as it is now.
-	// Then in the tree (in each node) I'd remember where I am at the moment (e.g. here I'm in "sampler",
-	// so I don't need buckets). It'd enable some custom handling for another weird types of requests.
-
-	if query.Aggregators[aggregatorsLevel].Filters {
-		subResult["buckets"] = bucketsReturnMap[0]
-	} else if query.Aggregators[aggregatorsLevel].Keyed {
-		subResult["buckets"] = bucketsReturnMap[0]
-	} else if query.Aggregators[aggregatorsLevel].SplitOverHowManyFields == 0 {
-		subResult = bucketsReturnMap[0]
-	} else {
 		subResult["buckets"] = bucketsReturnMap
 	}
 
 	_ = cw.addMetadataIfNeeded(query, subResult, aggregatorsLevel)
 
-	result[query.Aggregators[aggregatorsLevel].Name] = subResult
-	return []model.JsonMap{result}
+	return []model.JsonMap{{
+		currentAggregator.Name: subResult,
+	}}
 }
 
 // addMetadataIfNeeded adds metadata to the `result` dictionary, if needed.