Skip to content

Commit

Permalink
Remove unused relic model.KeyAddedByQuesma (#839)
Browse files Browse the repository at this point in the history
Wasn't used anymore anywhere.
  • Loading branch information
trzysiek authored Oct 3, 2024
1 parent b3f48a9 commit f43e024
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 23 deletions.
2 changes: 0 additions & 2 deletions quesma/model/query_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ type QueryResultRow struct {
Cols []QueryResultCol
}

const KeyAddedByQuesma = "Quesma_key_JR*#@(DF*GAsFfS!/LI" // created in a way that there shouldn't be a field of this name

// String returns the string representation of the column in format `"<colName>": <value>`, properly quoted.
func (c QueryResultCol) String(ctx context.Context) string {
valueExtracted := c.ExtractValue(ctx)
Expand Down
2 changes: 1 addition & 1 deletion quesma/queryparser/aggregation_parser_new_logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func Test3AggregationParserNewLogic(t *testing.T) {
}

// probability and seed are present in random_sampler aggregation. I'd assume they are not needed, thus let's not care about it for now.
acceptableDifference := []string{"sum_other_doc_count", "probability", "seed", "bg_count", "doc_count", model.KeyAddedByQuesma,
acceptableDifference := []string{"sum_other_doc_count", "probability", "seed", "bg_count", "doc_count",
"sum_other_doc_count", "doc_count_error_upper_bound"} // Don't know why, but those 2 are still needed in new (clients/ophelia) tests. Let's fix it in another PR

actualMinusExpected, expectedMinusActual := util.MapDifference(response.Aggregations,
Expand Down
9 changes: 4 additions & 5 deletions quesma/queryparser/pancake_json_rendering.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (p *pancakeJSONRenderer) combinatorBucketToJSON(remainingLayers []*pancakeM
if err != nil {
return nil, err
}
return util.MergeMaps(p.ctx, aggJson, subAggr, model.KeyAddedByQuesma), nil
return util.MergeMaps(p.ctx, aggJson, subAggr), nil
case bucket_aggregations.CombinatorAggregationInterface:
var bucketArray []model.JsonMap
for _, subGroup := range queryType.CombinatorGroups() {
Expand All @@ -195,7 +195,7 @@ func (p *pancakeJSONRenderer) combinatorBucketToJSON(remainingLayers []*pancakeM
aggJson := queryType.CombinatorTranslateSqlResponseToJson(subGroup, selectedRows)

bucketArray = append(bucketArray,
util.MergeMaps(p.ctx, aggJson, subAggr, model.KeyAddedByQuesma))
util.MergeMaps(p.ctx, aggJson, subAggr))
bucketArray[len(bucketArray)-1]["key"] = subGroup.Key
}
var bucketsJson any
Expand Down Expand Up @@ -296,12 +296,11 @@ func (p *pancakeJSONRenderer) layerToJSON(remainingLayers []*pancakeModelLayer,
continue
}

// TODO: Maybe add model.KeyAddedByQuesma if there are more than one pancake
subAggr, err := p.layerToJSON(remainingLayers[1:], subAggrRows[i])
if err != nil {
return nil, err
}
bucketArr[i] = util.MergeMaps(p.ctx, bucket, subAggr, model.KeyAddedByQuesma)
bucketArr[i] = util.MergeMaps(p.ctx, bucket, subAggr)
}
} else {
// A bit harder case. Observation: len(bucketArr) > len(subAggrRows) and set(subAggrRows' keys) is a subset of set(bucketArr's keys)
Expand Down Expand Up @@ -330,7 +329,7 @@ func (p *pancakeJSONRenderer) layerToJSON(remainingLayers []*pancakeModelLayer,
if err != nil {
return nil, err
}
bucketArr[i] = util.MergeMaps(p.ctx, bucket, subAggr, model.KeyAddedByQuesma)
bucketArr[i] = util.MergeMaps(p.ctx, bucket, subAggr)
subAggrIdx++
} else {
bucketArr[i] = bucket
Expand Down
3 changes: 1 addition & 2 deletions quesma/queryparser/pancake_sql_query_generation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ func TestPancakeQueryGeneration(t *testing.T) {
}

// FIXME we can quite easily remove 'probability' and 'seed' from above - just start remembering them in RandomSampler struct and print in JSON response.
acceptableDifference := []string{"probability", "seed", "bg_count", model.KeyAddedByQuesma,
"doc_count_error_upper_bound"} // Don't know why, but those 2 are still needed in new (clients/ophelia) tests. Let's fix it in another PR
acceptableDifference := []string{"probability", "seed", "bg_count", "doc_count_error_upper_bound"} // Don't know why, but those 2 are still needed in new (clients/ophelia) tests. Let's fix it in another PR
if len(test.AdditionalAcceptableDifference) > 0 {
acceptableDifference = append(acceptableDifference, test.AdditionalAcceptableDifference...)
}
Expand Down
2 changes: 1 addition & 1 deletion quesma/queryparser/query_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (cw *ClickhouseQueryTranslator) MakeAggregationPartOfResponse(queries []*mo
return nil, err
}

aggregations = util.MergeMaps(cw.Ctx, aggregations, aggregation, "key")
aggregations = util.MergeMaps(cw.Ctx, aggregations, aggregation)
}
}
return aggregations, nil
Expand Down
3 changes: 1 addition & 2 deletions quesma/quesma/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,7 @@ func TestNumericFacetsQueries(t *testing.T) {
responsePart = responseMap["response"].(model.JsonMap)
}

acceptableDifference := []string{"probability", "seed", "bg_count", model.KeyAddedByQuesma,
"doc_count_error_upper_bound", "__quesma_total_count"}
acceptableDifference := []string{"probability", "seed", "bg_count", "doc_count_error_upper_bound", "__quesma_total_count"}
expectedJson := types.MustJSON(tt.ResultJson)["response"].(model.JsonMap)

// Eventually we should remove two below lines
Expand Down
18 changes: 9 additions & 9 deletions quesma/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func JsonDifference(jsonActual, jsonExpected string) (JsonMap, JsonMap, error) {
// but none of them works for nested maps, so needed to write our own.
// * mActual - uses JsonMap fully: values are []JsonMap, or JsonMap, or base types
// * mExpected - value can also be []any, because it's generated from Golang's json.Unmarshal
func MergeMaps(ctx context.Context, mActual, mExpected JsonMap, keyAddedByQuesma string) JsonMap {
func MergeMaps(ctx context.Context, mActual, mExpected JsonMap) JsonMap {
var mergeMapsRec func(m1, m2 JsonMap) JsonMap
// merges 'i1' and 'i2' in 3 cases: both are JsonMap, both are []JsonMap, or both are some base type
mergeAny := func(i1, i2 any) any {
Expand Down Expand Up @@ -301,13 +301,13 @@ func MergeMaps(ctx context.Context, mActual, mExpected JsonMap, keyAddedByQuesma
i, j := 0, 0
for i < i1Len && j < i2Len {
var key1, key2 string
key1, ok = i1Typed[i][keyAddedByQuesma].(string) // TODO maybe some other types as well?
key1, ok = i1Typed[i]["key"].(string) // TODO maybe some other types as well?
if !ok {
if key1Int, ok := i1Typed[i][keyAddedByQuesma].(int64); ok {
if key1Int, ok := i1Typed[i]["key"].(int64); ok {
key1 = strconv.FormatInt(key1Int, 10)
} else if key1Uint, ok := i1Typed[i][keyAddedByQuesma].(uint64); ok {
} else if key1Uint, ok := i1Typed[i]["key"].(uint64); ok {
key1 = strconv.FormatUint(key1Uint, 10)
} else if key1Float, ok := i1Typed[i][keyAddedByQuesma].(float64); ok {
} else if key1Float, ok := i1Typed[i]["key"].(float64); ok {
key1 = strconv.FormatFloat(key1Float, 'f', -1, 64)
} else {
// TODO keys probably can be other types, e.g. bools
Expand All @@ -316,13 +316,13 @@ func MergeMaps(ctx context.Context, mActual, mExpected JsonMap, keyAddedByQuesma
continue
}
}
key2, ok = i2Typed[j].(JsonMap)[keyAddedByQuesma].(string) // TODO maybe some other types as well?
key2, ok = i2Typed[j].(JsonMap)["key"].(string) // TODO maybe some other types as well?
if !ok {
if key2Int, ok := i2Typed[j].(JsonMap)[keyAddedByQuesma].(int64); ok {
if key2Int, ok := i2Typed[j].(JsonMap)["key"].(int64); ok {
key2 = strconv.FormatInt(key2Int, 10)
} else if key2Uint, ok := i2Typed[j].(JsonMap)[keyAddedByQuesma].(uint64); ok {
} else if key2Uint, ok := i2Typed[j].(JsonMap)["key"].(uint64); ok {
key2 = strconv.FormatUint(key2Uint, 10)
} else if key2Float, ok := i2Typed[j].(JsonMap)[keyAddedByQuesma].(float64); ok {
} else if key2Float, ok := i2Typed[j].(JsonMap)["key"].(float64); ok {
key2 = strconv.FormatFloat(key2Float, 'f', -1, 64)
} else {
// TODO keys probably can be other types, e.g. bools
Expand Down
2 changes: 1 addition & 1 deletion quesma/util/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ func TestMergeMaps(t *testing.T) {
for i, tt := range cases {
t.Run("TestMergeMaps_"+strconv.Itoa(i), func(t *testing.T) {
// simple == or Equal doesn't work on nested maps => need DeepEqual
assert.True(t, reflect.DeepEqual(tt.wanted, MergeMaps(context.Background(), tt.m1, tt.m2, "not-important")))
assert.True(t, reflect.DeepEqual(tt.wanted, MergeMaps(context.Background(), tt.m1, tt.m2)))
})
}
}
Expand Down

0 comments on commit f43e024

Please sign in to comment.