Skip to content

Commit

Permalink
Remove clickhouse.LogManager from ClickhouseQueryTranslator (#1117)
Browse files Browse the repository at this point in the history
This instance is not really needed, `HandleTermsEnum` was the only place
where this has been used.

The biggest change is probably `terms_enum.go` and also the
`ResolveField(cw.Ctx, fieldStr, cw.Schema)` - it's no longer a method,
but function taking Schema as an argument.
  • Loading branch information
mieciu authored Dec 17, 2024
1 parent 047ed16 commit 526e5c1
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 78 deletions.
4 changes: 2 additions & 2 deletions quesma/queryparser/aggregation_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (cw *ClickhouseQueryTranslator) parseFieldField(shouldBeMap any, aggregatio
}
if fieldRaw, ok := Map["field"]; ok {
if field, ok := fieldRaw.(string); ok {
return model.NewColumnRef(cw.ResolveField(cw.Ctx, field)) // model.NewSelectColumnTableField(cw.Table.ResolveField(cw.Ctx, field)) // remove this resolve? we do all transforms after parsing is done?
return model.NewColumnRef(ResolveField(cw.Ctx, field, cw.Schema)) // model.NewSelectColumnTableField(cw.Table.ResolveField(cw.Ctx, field)) // remove this resolve? we do all transforms after parsing is done?
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("field is not a string, but %T, value: %v", fieldRaw, fieldRaw)
}
Expand Down Expand Up @@ -322,7 +322,7 @@ func (cw *ClickhouseQueryTranslator) parseFieldFieldMaybeScript(shouldBeMap any,
// maybe "field" field
if fieldRaw, ok := Map["field"]; ok {
if field, ok := fieldRaw.(string); ok {
return model.NewColumnRef(cw.ResolveField(cw.Ctx, field)), true // remove this resolve? we do all transforms after parsing is done?
return model.NewColumnRef(ResolveField(cw.Ctx, field, cw.Schema)), true // remove this resolve? we do all transforms after parsing is done?
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("field is not a string, but %T, value: %v", fieldRaw, fieldRaw)
}
Expand Down
4 changes: 1 addition & 3 deletions quesma/queryparser/aggregation_parser_new_logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/assert"
"quesma/clickhouse"
"quesma/model"
"quesma/quesma/config"
"quesma/quesma/types"
"quesma/schema"
"quesma/testdata"
Expand All @@ -34,7 +33,6 @@ func Test3AggregationParserNewLogic(t *testing.T) {
Name: tableName,
Config: clickhouse.NewDefaultCHConfig(),
}
lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, &table), &config.QuesmaConfiguration{})

s := schema.StaticRegistry{
Tables: map[schema.IndexName]schema.Schema{
Expand All @@ -54,7 +52,7 @@ func Test3AggregationParserNewLogic(t *testing.T) {
},
},
}
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background(), Schema: s.Tables[schema.IndexName(tableName)]}
cw := ClickhouseQueryTranslator{Table: &table, Ctx: context.Background(), Schema: s.Tables[schema.IndexName(tableName)]}

for i, test := range testdata.NewLogicTestCases {
t.Run(test.TestName+"("+strconv.Itoa(i)+")", func(t *testing.T) {
Expand Down
8 changes: 2 additions & 6 deletions quesma/queryparser/pancake_sql_query_generation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"quesma/clickhouse"
"quesma/model"
"quesma/model/bucket_aggregations"
"quesma/quesma/config"
"quesma/quesma/types"
"quesma/schema"
"quesma/util"
Expand Down Expand Up @@ -38,15 +37,14 @@ func TestPancakeQueryGeneration(t *testing.T) {
Config: clickhouse.NewDefaultCHConfig(),
}

lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, &table), &config.QuesmaConfiguration{})
currentSchema := schema.Schema{
Fields: nil,
Aliases: nil,
ExistsInDataSource: false,
DatabaseName: "",
}

cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background(), Schema: currentSchema}
cw := ClickhouseQueryTranslator{Table: &table, Ctx: context.Background(), Schema: currentSchema}

for i, test := range allAggregationTests() {
t.Run(test.TestName+"("+strconv.Itoa(i)+")", func(t *testing.T) {
Expand Down Expand Up @@ -211,11 +209,9 @@ func TestPancakeQueryGeneration_halfpancake(t *testing.T) {
Config: clickhouse.NewDefaultCHConfig(),
}

lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, &table), &config.QuesmaConfiguration{})

currentSchema := schema.Schema{}

cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background(), Schema: currentSchema}
cw := ClickhouseQueryTranslator{Table: &table, Ctx: context.Background(), Schema: currentSchema}

tests := []struct {
name string
Expand Down
33 changes: 16 additions & 17 deletions quesma/queryparser/query_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"quesma/model/typical_queries"
"quesma/queryparser/lucene"
"quesma/quesma/types"
"quesma/schema"
"quesma/util"
"strconv"
"strings"
Expand Down Expand Up @@ -232,7 +233,7 @@ func (cw *ClickhouseQueryTranslator) parseMetadata(queryMap QueryMap) QueryMap {
}

func (cw *ClickhouseQueryTranslator) ParseAutocomplete(indexFilter *QueryMap, fieldName string, prefix *string, caseIns bool) model.SimpleQuery {
fieldName = cw.ResolveField(cw.Ctx, fieldName)
fieldName = ResolveField(cw.Ctx, fieldName, cw.Schema)
canParse := true
stmts := make([]model.Expr, 0)
if indexFilter != nil {
Expand Down Expand Up @@ -474,7 +475,7 @@ func (cw *ClickhouseQueryTranslator) parseTerm(queryMap QueryMap) model.SimpleQu
whereClause = model.NewInfixExpr(model.NewLiteral("0"), "=", model.NewLiteral("0 /* "+k+"="+sprint(v)+" */"))
return model.NewSimpleQuery(whereClause, true)
}
fieldName := cw.ResolveField(cw.Ctx, k)
fieldName := ResolveField(cw.Ctx, k, cw.Schema)
whereClause = model.NewInfixExpr(model.NewColumnRef(fieldName), "=", model.NewLiteral(sprint(v)))
return model.NewSimpleQuery(whereClause, true)
}
Expand Down Expand Up @@ -540,7 +541,7 @@ func (cw *ClickhouseQueryTranslator) parseMatch(queryMap QueryMap, matchPhrase b
}

for fieldName, v := range queryMap {
fieldName = cw.ResolveField(cw.Ctx, fieldName)
fieldName = ResolveField(cw.Ctx, fieldName, cw.Schema)
// (fieldName, v) = either e.g. ("message", "this is a test")
// or ("message", map["query": "this is a test", ...]). Here we only care about "query" until we find a case where we need more.
vUnNested := v
Expand Down Expand Up @@ -640,7 +641,7 @@ func (cw *ClickhouseQueryTranslator) parsePrefix(queryMap QueryMap) model.Simple
}

for fieldName, v := range queryMap {
fieldName = cw.ResolveField(cw.Ctx, fieldName)
fieldName = ResolveField(cw.Ctx, fieldName, cw.Schema)
switch vCasted := v.(type) {
case string:
simpleStat := model.NewInfixExpr(model.NewColumnRef(fieldName), "iLIKE", model.NewLiteral("'"+vCasted+"%'"))
Expand Down Expand Up @@ -670,7 +671,7 @@ func (cw *ClickhouseQueryTranslator) parseWildcard(queryMap QueryMap) model.Simp
}

for fieldName, v := range queryMap {
fieldName = cw.ResolveField(cw.Ctx, fieldName)
fieldName = ResolveField(cw.Ctx, fieldName, cw.Schema)
if vAsMap, ok := v.(QueryMap); ok {
if value, ok := vAsMap["value"]; ok {
if valueAsString, ok := value.(string); ok {
Expand Down Expand Up @@ -762,9 +763,9 @@ func (cw *ClickhouseQueryTranslator) parseRange(queryMap QueryMap) model.SimpleQ
const dateInSchemaExpected = true

for fieldName, v := range queryMap {
fieldName = cw.ResolveField(cw.Ctx, fieldName)
fieldName = ResolveField(cw.Ctx, fieldName, cw.Schema)

fieldType := cw.Table.GetDateTimeType(cw.Ctx, cw.ResolveField(cw.Ctx, fieldName), dateInSchemaExpected)
fieldType := cw.Table.GetDateTimeType(cw.Ctx, ResolveField(cw.Ctx, fieldName, cw.Schema), dateInSchemaExpected)
stmts := make([]model.Expr, 0)
if _, ok := v.(QueryMap); !ok {
logger.WarnWithCtx(cw.Ctx).Msgf("invalid range type: %T, value: %v", v, v)
Expand Down Expand Up @@ -943,7 +944,7 @@ func (cw *ClickhouseQueryTranslator) extractFields(fields []interface{}) []strin
if fieldStr == "*" {
return []string{model.FullTextFieldNamePlaceHolder}
}
fieldStr = cw.ResolveField(cw.Ctx, fieldStr)
fieldStr = ResolveField(cw.Ctx, fieldStr, cw.Schema)
result = append(result, fieldStr)
}
return result
Expand Down Expand Up @@ -1044,7 +1045,7 @@ func (cw *ClickhouseQueryTranslator) isItListRequest(queryMap QueryMap) (model.H
}
}

resolvedField := cw.ResolveField(cw.Ctx, fieldName)
resolvedField := ResolveField(cw.Ctx, fieldName, cw.Schema)
if resolvedField == "*" {
return model.HitsCountInfo{Typ: model.ListAllFields, RequestedFields: []string{"*"}, Size: size}, true
}
Expand Down Expand Up @@ -1093,11 +1094,11 @@ func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps any) (sortColumns
// sortMap has only 1 key, so we can just iterate over it
for k, v := range sortMap {
// TODO replace cw.Table.GetFieldInfo with schema.Field[]
if strings.HasPrefix(k, "_") && cw.Table.GetFieldInfo(cw.Ctx, cw.ResolveField(cw.Ctx, k)) == clickhouse.NotExists {
if strings.HasPrefix(k, "_") && cw.Table.GetFieldInfo(cw.Ctx, ResolveField(cw.Ctx, k, cw.Schema)) == clickhouse.NotExists {
// we're skipping ELK internal fields, like "_doc", "_id", etc.
continue
}
fieldName := cw.ResolveField(cw.Ctx, k)
fieldName := ResolveField(cw.Ctx, k, cw.Schema)
switch v := v.(type) {
case QueryMap:
if order, ok := v["order"]; ok {
Expand Down Expand Up @@ -1127,7 +1128,7 @@ func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps any) (sortColumns
return sortColumns
case map[string]interface{}:
for fieldName, fieldValue := range sortMaps {
if strings.HasPrefix(fieldName, "_") && cw.Table.GetFieldInfo(cw.Ctx, cw.ResolveField(cw.Ctx, fieldName)) == clickhouse.NotExists {
if strings.HasPrefix(fieldName, "_") && cw.Table.GetFieldInfo(cw.Ctx, ResolveField(cw.Ctx, fieldName, cw.Schema)) == clickhouse.NotExists {
// TODO Elastic internal fields will need to be supported in the future
continue
}
Expand All @@ -1144,7 +1145,7 @@ func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps any) (sortColumns

case map[string]string:
for fieldName, fieldValue := range sortMaps {
if strings.HasPrefix(fieldName, "_") && cw.Table.GetFieldInfo(cw.Ctx, cw.ResolveField(cw.Ctx, fieldName)) == clickhouse.NotExists {
if strings.HasPrefix(fieldName, "_") && cw.Table.GetFieldInfo(cw.Ctx, ResolveField(cw.Ctx, fieldName, cw.Schema)) == clickhouse.NotExists {
// TODO Elastic internal fields will need to be supported in the future
continue
}
Expand Down Expand Up @@ -1180,11 +1181,9 @@ func createSortColumn(fieldName, ordering string) (model.OrderByExpr, error) {
// What prevents us from moving it to transformation pipeline now, is that
// we need to anotate this field somehow in the AST, to be able
// to distinguish it from other fields
func (cw *ClickhouseQueryTranslator) ResolveField(ctx context.Context, fieldName string) string {
func ResolveField(ctx context.Context, fieldName string, schemaInstance schema.Schema) string {
// Alias resolution should occur *after* the query is parsed, not during the parsing

schemaInstance := cw.Schema

fieldName = strings.TrimSuffix(fieldName, ".keyword")
fieldName = strings.TrimSuffix(fieldName, ".text")

Expand Down Expand Up @@ -1221,7 +1220,7 @@ func (cw *ClickhouseQueryTranslator) parseSize(queryMap QueryMap, defaultSize in
func (cw *ClickhouseQueryTranslator) GetDateTimeTypeFromSelectClause(ctx context.Context, expr model.Expr,
dateInSchemaExpected bool) clickhouse.DateTimeType {
if ref, ok := expr.(model.ColumnRef); ok {
return cw.Table.GetDateTimeType(ctx, cw.ResolveField(ctx, ref.ColumnName), dateInSchemaExpected)
return cw.Table.GetDateTimeType(ctx, ResolveField(ctx, ref.ColumnName, cw.Schema), dateInSchemaExpected)
}
return clickhouse.Invalid
}
Expand Down
5 changes: 1 addition & 4 deletions quesma/queryparser/query_parser_range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import (
"context"
"github.com/stretchr/testify/assert"
"quesma/clickhouse"
"quesma/quesma/config"
"quesma/schema"
"quesma/util"
"testing"
)

Expand Down Expand Up @@ -102,8 +100,7 @@ func Test_parseRange(t *testing.T) {
t.Fatal(err)
}
assert.NoError(t, err)
lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, table), &config.QuesmaConfiguration{})
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background(), Schema: s.Tables[schema.IndexName(tableName)]}
cw := ClickhouseQueryTranslator{Table: table, Ctx: context.Background(), Schema: s.Tables[schema.IndexName(tableName)]}

simpleQuery := cw.parseRange(test.rangePartOfQuery)
assert.Equal(t, test.expectedWhere, simpleQuery.WhereClauseAsString())
Expand Down
13 changes: 5 additions & 8 deletions quesma/queryparser/query_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestQueryParserStringAttrConfig(t *testing.T) {
},
},
}
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background(), Config: &cfg, Schema: s.Tables[schema.IndexName(tableName)]}
cw := ClickhouseQueryTranslator{Table: table, Ctx: context.Background(), Config: &cfg, Schema: s.Tables[schema.IndexName(tableName)]}

for i, tt := range testdata.TestsSearch {
t.Run(fmt.Sprintf("%s(%d)", tt.Name, i), func(t *testing.T) {
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestQueryParserNoFullTextFields(t *testing.T) {
},
},
}
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background(), Config: &cfg, Schema: s.Tables[schema.IndexName(tableName)]}
cw := ClickhouseQueryTranslator{Table: &table, Ctx: context.Background(), Config: &cfg, Schema: s.Tables[schema.IndexName(tableName)]}

for i, tt := range testdata.TestsSearchNoFullTextFields {
t.Run(strconv.Itoa(i), func(t *testing.T) {
Expand Down Expand Up @@ -185,8 +185,7 @@ func TestQueryParserNoAttrsConfig(t *testing.T) {
},
},
}
lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, table), &config.QuesmaConfiguration{})
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background(), Config: &cfg, Schema: s.Tables["logs-generic-default"]}
cw := ClickhouseQueryTranslator{Table: table, Ctx: context.Background(), Config: &cfg, Schema: s.Tables["logs-generic-default"]}
for _, tt := range testdata.TestsSearchNoAttrs {
t.Run(tt.Name, func(t *testing.T) {
body, parseErr := types.ParseJSON(tt.QueryJson)
Expand Down Expand Up @@ -269,8 +268,7 @@ func Test_parseSortFields(t *testing.T) {
ENGINE = Memory`,
clickhouse.NewChTableConfigNoAttrs(),
)
lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, table), &config.QuesmaConfiguration{})
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background()}
cw := ClickhouseQueryTranslator{Table: table, Ctx: context.Background()}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.sortColumns, cw.parseSortFields(tt.sortMap))
Expand All @@ -294,15 +292,14 @@ func TestInvalidQueryRequests(t *testing.T) {
Config: clickhouse.NewDefaultCHConfig(),
}

lm := clickhouse.NewLogManager(util.NewSyncMapWith(tableName, &table), &config.QuesmaConfiguration{})
currentSchema := schema.Schema{
Fields: nil,
Aliases: nil,
ExistsInDataSource: false,
DatabaseName: "",
}

cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background(), Schema: currentSchema}
cw := ClickhouseQueryTranslator{Table: &table, Ctx: context.Background(), Schema: currentSchema}

for i, test := range testdata.InvalidAggregationTests {
t.Run(test.TestName+"("+strconv.Itoa(i)+")", func(t *testing.T) {
Expand Down
19 changes: 0 additions & 19 deletions quesma/queryparser/query_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (
type JsonMap = map[string]interface{}

type ClickhouseQueryTranslator struct {
ClickhouseLM *clickhouse.LogManager

Schema schema.Schema
Ctx context.Context

Expand Down Expand Up @@ -312,20 +310,3 @@ func (cw *ClickhouseQueryTranslator) BuildCountQuery(whereClause model.Expr, sam
func (cw *ClickhouseQueryTranslator) BuildNRowsQuery(fieldNames []string, query *model.SimpleQuery, limit int) *model.Query {
return query_util.BuildHitsQuery(cw.Ctx, model.SingleTableNamePlaceHolder, fieldNames, query, limit)
}

func (cw *ClickhouseQueryTranslator) BuildAutocompleteQuery(fieldName, tableName string, whereClause model.Expr, limit int) *model.Query {
return &model.Query{
SelectCommand: *model.NewSelectCommand(
[]model.Expr{model.NewColumnRef(fieldName)},
nil,
nil,
model.NewTableRef(tableName),
whereClause,
[]model.Expr{},
limit,
0,
true,
nil,
),
}
}
2 changes: 1 addition & 1 deletion quesma/queryparser/query_translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func TestMakeResponseAsyncSearchQuery(t *testing.T) {
// tests MakeSearchResponse, in particular if JSON we return is a proper JSON.
// used to fail before we fixed field quoting.
func TestMakeResponseSearchQueryIsProperJson(t *testing.T) {
cw := ClickhouseQueryTranslator{ClickhouseLM: nil, Table: clickhouse.NewEmptyTable("@"), Ctx: context.Background()}
cw := ClickhouseQueryTranslator{Table: clickhouse.NewEmptyTable("@"), Ctx: context.Background()}
const limit = 1000
queries := []*model.Query{
cw.BuildNRowsQuery([]string{"*"}, &model.SimpleQuery{}, limit),
Expand Down
30 changes: 23 additions & 7 deletions quesma/quesma/functionality/terms_enum/terms_enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,11 @@ func HandleTermsEnum(ctx context.Context, index string, body types.JSON, lm *cli
return []byte{}, end_user_errors.ErrNoSuchSchema.New(fmt.Errorf("can't load %s schema", resolvedTableName)).Details("Table: %s", resolvedTableName)
}

return handleTermsEnumRequest(ctx, body, &queryparser.ClickhouseQueryTranslator{
ClickhouseLM: lm, Table: lm.FindTable(indices[0]), Ctx: context.Background(), Schema: resolvedSchema,
}, qmc)
return handleTermsEnumRequest(ctx, body, lm, &queryparser.ClickhouseQueryTranslator{Table: lm.FindTable(indices[0]), Ctx: context.Background(), Schema: resolvedSchema}, qmc)
}
}

func handleTermsEnumRequest(ctx context.Context, body types.JSON, qt *queryparser.ClickhouseQueryTranslator,
func handleTermsEnumRequest(ctx context.Context, body types.JSON, lm *clickhouse.LogManager, qt *queryparser.ClickhouseQueryTranslator,
qmc diag.DebugInfoCollector) (result []byte, err error) {
startTime := time.Now()

Expand All @@ -60,7 +58,7 @@ func handleTermsEnumRequest(ctx context.Context, body types.JSON, qt *queryparse
logger.ErrorWithCtx(ctx).Msgf("error reading terms enum API request body: field is not present")
return json.Marshal(emptyTermsEnumResponse())
}
field = qt.ResolveField(ctx, field)
field = queryparser.ResolveField(ctx, field, qt.Schema)

size := defaultSize
if sizeRaw, ok := body["size"]; ok {
Expand Down Expand Up @@ -93,11 +91,12 @@ func handleTermsEnumRequest(ctx context.Context, body types.JSON, qt *queryparse
}

where := qt.ParseAutocomplete(indexFilter, field, prefixString, caseInsensitive)
selectQuery := qt.BuildAutocompleteQuery(field, qt.Table.Name, where.WhereClause, size)
selectQuery := buildAutocompleteQuery(field, qt.Table.Name, where.WhereClause, size)
dbQueryCtx, cancel := context.WithCancel(ctx)
// TODO this will be used to cancel goroutine that is executing the query
_ = cancel
if rows, _, err2 := qt.ClickhouseLM.ProcessQuery(dbQueryCtx, qt.Table, selectQuery); err2 != nil {

if rows, _, err2 := lm.ProcessQuery(dbQueryCtx, qt.Table, selectQuery); err2 != nil {
logger.Error().Msgf("terms enum failed - error processing SQL query [%s]", err2)
result, err = json.Marshal(emptyTermsEnumResponse())
} else {
Expand Down Expand Up @@ -149,3 +148,20 @@ func emptyTermsEnumResponse() *model.TermsEnumResponse {
Terms: nil,
}
}

func buildAutocompleteQuery(fieldName, tableName string, whereClause model.Expr, limit int) *model.Query {
return &model.Query{
SelectCommand: *model.NewSelectCommand(
[]model.Expr{model.NewColumnRef(fieldName)},
nil,
nil,
model.NewTableRef(tableName),
whereClause,
[]model.Expr{},
limit,
0,
true,
nil,
),
}
}
Loading

0 comments on commit 526e5c1

Please sign in to comment.