Unify hits and hitsPresent #163

- `BuildSimpleSelectQuery` is unused, but tested. We should run `BuildNRowsQuery` instead - Unify `hits` and `hitsPresent` logic
QuesmaOrg · May 20, 2024 · 81e478d · 81e478d
1 parent 602571b
commit 81e478d
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 35 deletions.
diff --git a/quesma/queryparser/query_parser_test.go b/quesma/queryparser/query_parser_test.go
@@ -52,7 +52,7 @@ func TestQueryParserStringAttrConfig(t *testing.T) {
 			assert.True(t, simpleQuery.CanParse, "can parse")
 			assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql")
 			assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type")
-			query := cw.BuildSimpleSelectQuery(simpleQuery.Sql.Stmt, model.DefaultSizeListQuery)
+			query := cw.BuildNRowsQuery("*", simpleQuery, model.DefaultSizeListQuery)
 			assert.Contains(t, tt.WantedQuery, *query)
 		})
 	}
@@ -79,7 +79,7 @@ func TestQueryParserNoFullTextFields(t *testing.T) {
 			assert.True(t, simpleQuery.CanParse, "can parse")
 			assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql")
 			assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type")
-			query := cw.BuildSimpleSelectQuery(simpleQuery.Sql.Stmt, model.DefaultSizeListQuery)
+			query := cw.BuildNRowsQuery("*", simpleQuery, model.DefaultSizeListQuery)
 			assert.Contains(t, tt.WantedQuery, *query)
 		})
 	}
@@ -105,7 +105,7 @@ func TestQueryParserNoAttrsConfig(t *testing.T) {
 			assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt)
 			assert.Equal(t, tt.WantedQueryType, queryInfo.Typ)
 
-			query := cw.BuildSimpleSelectQuery(simpleQuery.Sql.Stmt, model.DefaultSizeListQuery)
+			query := cw.BuildNRowsQuery("*", simpleQuery, model.DefaultSizeListQuery)
 			assert.Contains(t, tt.WantedQuery, *query)
 		})
 	}

diff --git a/quesma/queryparser/query_translator.go b/quesma/queryparser/query_translator.go
@@ -552,16 +552,6 @@ func (cw *ClickhouseQueryTranslator) BuildSelectQuery(fields []string, whereClau
 	}
 }
 
-func (cw *ClickhouseQueryTranslator) BuildSimpleSelectQuery(whereClause string, limit int) *model.Query {
-	return &model.Query{
-		Fields:        []string{"*"},
-		WhereClause:   whereClause,
-		FromClause:    cw.Table.FullTableName(),
-		SuffixClauses: []string{"LIMIT " + strconv.Itoa(cw.applySizeLimit(limit))},
-		CanParse:      true,
-	}
-}
-
 func (cw *ClickhouseQueryTranslator) BuildSimpleCountQuery(whereClause string) *model.Query {
 	return &model.Query{
 		NonSchemaFields: []string{"count()"},
@@ -592,12 +582,11 @@ func (cw *ClickhouseQueryTranslator) BuildNRowsQuery(fieldName string, query Sim
 		suffixClauses = append(suffixClauses, "LIMIT "+strconv.Itoa(cw.applySizeLimit(limit)))
 	}
 	return &model.Query{
-		Fields:          []string{fieldName},
-		NonSchemaFields: []string{},
-		WhereClause:     query.Sql.Stmt,
-		SuffixClauses:   suffixClauses,
-		FromClause:      cw.Table.FullTableName(),
-		CanParse:        true,
+		Fields:        []string{fieldName},
+		WhereClause:   query.Sql.Stmt,
+		SuffixClauses: suffixClauses,
+		FromClause:    cw.Table.FullTableName(),
+		CanParse:      true,
 	}
 }
 

diff --git a/quesma/queryparser/query_translator_test.go b/quesma/queryparser/query_translator_test.go
@@ -464,7 +464,7 @@ func TestMakeResponseSearchQueryIsProperJson(t *testing.T) {
 	cw := ClickhouseQueryTranslator{ClickhouseLM: nil, Table: clickhouse.NewEmptyTable("@"), Ctx: context.Background()}
 	const limit = 1000
 	queries := []*model.Query{
-		cw.BuildSimpleSelectQuery("", limit),
+		cw.BuildNRowsQuery("*", SimpleQuery{}, limit),
 		cw.BuildNRowsQuery("@", SimpleQuery{}, 0),
 	}
 	for _, query := range queries {

diff --git a/quesma/quesma/search.go b/quesma/quesma/search.go
@@ -189,7 +189,6 @@ func (q *QueryRunner) handleSearchCommon(ctx context.Context, indexPattern strin
 	}
 
 	var hits []model.QueryResultRow
-	var hitsPresent *[]model.QueryResultRow
 	var aggregationResults [][]model.QueryResultRow
 	oldHandlingUsed := false
 	newAggregationHandlingUsed := false
@@ -270,8 +269,7 @@ func (q *QueryRunner) handleSearchCommon(ctx context.Context, indexPattern strin
 					translatedQueryBody, aggregationResults = q.searchWorker(ctx, aggregations, columns, table, true, nil)
 					if queryInfo.Size > 0 {
 						listQuery := queryTranslator.BuildNRowsQuery("*", simpleQuery, queryInfo.Size)
-						hitsFallback, err := q.logManager.ProcessQuery(ctx, table, listQuery, nil)
-						hitsPresent = &hitsFallback
+						hits, err = q.logManager.ProcessQuery(ctx, table, listQuery, nil)
 						translatedQueryBody = append(translatedQueryBody, []byte("\n"+listQuery.String()+"\n")...)
 						if err != nil {
 							logger.ErrorWithCtx(ctx).Msgf("error processing fallback query. Err: %v, query: %+v", err, listQuery)
@@ -295,24 +293,15 @@ func (q *QueryRunner) handleSearchCommon(ctx context.Context, indexPattern strin
 				response, err = queryTranslator.MakeSearchResponse(hits, model.Query{QueryInfo: queryInfo, Highlighter: highlighter})
 			} else if newAggregationHandlingUsed {
 				response = queryTranslator.MakeResponseAggregation(aggregations, aggregationResults)
+				responseHits, err = queryTranslator.MakeSearchResponse(hits, model.Query{QueryInfo: queryInfo, Highlighter: highlighter})
+				response.Hits = responseHits.Hits
 			}
 			if err != nil {
 				logger.ErrorWithCtx(ctx).Msgf("error making response: %v, queryInfo: %+v, rows: %v", err, queryInfo, hits)
 				pushSecondaryInfo(q.quesmaManagementConsole, id, path, body, translatedQueryBody, responseBody, startTime)
 				return responseBody, err
 			}
 
-			if hitsPresent != nil {
-				if response == nil {
-					response, err = queryTranslator.MakeSearchResponse(*hitsPresent, model.Query{QueryInfo: queryInfo, Highlighter: highlighter})
-				} else {
-					responseHits, err = queryTranslator.MakeSearchResponse(*hitsPresent, model.Query{QueryInfo: queryInfo, Highlighter: highlighter})
-					response.Hits = responseHits.Hits
-				}
-			}
-			if err != nil {
-				logger.ErrorWithCtx(ctx).Msgf("error making response: %v, queryInfo: %v, rows: %v", err, queryInfo, hitsPresent)
-			}
 			responseBody, err = response.Marshal()
 
 			pushSecondaryInfo(q.quesmaManagementConsole, id, path, body, translatedQueryBody, responseBody, startTime)

diff --git a/quesma/quesma/search_test.go b/quesma/quesma/search_test.go
@@ -36,7 +36,7 @@ func TestNoAsciiTableName(t *testing.T) {
 	assert.Equal(t, "", simpleQuery.Sql.Stmt)
 	assert.Equal(t, model.Normal, queryInfo.Typ)
 	const Limit = 1000
-	query := queryTranslator.BuildSimpleSelectQuery(simpleQuery.Sql.Stmt, Limit)
+	query := queryTranslator.BuildNRowsQuery("*", simpleQuery, Limit)
 	assert.True(t, query.CanParse)
 	assert.Equal(t, fmt.Sprintf(`SELECT * FROM "%s" LIMIT %d`, tableName, Limit), query.String())
 }