Rewrite almost done

quesma/logger/log_with_throttling.go

@@ -25,3 +25,14 @@ func WarnWithCtxAndThrottling(ctx context.Context, aggrName, paramName, format s
 		throttleMap.Store(mapKey, time.Now())
 	}
 }
+
+// WarnWithThrottling - logs a warning message with throttling.
+// We only log once per throttleDuration for each warnName, so that we don't spam the logs.
+func WarnWithThrottling(warnName, format string, v ...any) {
+	timestamp, ok := throttleMap.Load(warnName)
+	weThrottle := ok && time.Since(timestamp) < throttleDuration
+	if !weThrottle {
+		Warn().Msgf(format, v...)
+		throttleMap.Store(warnName, time.Now())
+	}
+}

quesma/model/base_visitor.go

@@ -5,6 +5,7 @@ package model
 type BaseExprVisitor struct {
 	OverrideVisitFunction       func(b *BaseExprVisitor, e FunctionExpr) interface{}
 	OverrideVisitLiteral        func(b *BaseExprVisitor, l LiteralExpr) interface{}
+	OverrideVisitTuple          func(b *BaseExprVisitor, t TupleExpr) interface{}
 	OverrideVisitInfix          func(b *BaseExprVisitor, e InfixExpr) interface{}
 	OverrideVisitColumnRef      func(b *BaseExprVisitor, e ColumnRef) interface{}
 	OverrideVisitPrefixExpr     func(b *BaseExprVisitor, e PrefixExpr) interface{}
@@ -41,8 +42,16 @@ func (v *BaseExprVisitor) VisitLiteral(e LiteralExpr) interface{} {
 		return v.OverrideVisitLiteral(v, e)
 	}
 
-	return NewLiteralWithEscape(e.Value, e.LiteralAlreadyEscaped)
+	return NewLiteral(e.Value)
 }
+
+func (v *BaseExprVisitor) VisitTuple(e TupleExpr) interface{} {
+	if v.OverrideVisitTuple != nil {
+		return v.OverrideVisitTuple(v, e)
+	}
+	return NewTupleExpr(v.VisitChildren(e.Exprs)...)
+}
+
 func (v *BaseExprVisitor) VisitInfix(e InfixExpr) interface{} {
 	if v.OverrideVisitInfix != nil {
 		return v.OverrideVisitInfix(v, e)

quesma/model/expr.go

@@ -80,15 +80,24 @@ func (e FunctionExpr) Accept(v ExprVisitor) interface{} {
 
 type LiteralExpr struct {
 	Value any
-	// LiteralAlreadyEscaped in 99.5% will simply be false, which is the default.
-	// So far it's only true in terms Query DSL, e.g. "field IN ('abc', 'def')". There we don't want to escape "('abc', 'def')" further.
-	LiteralAlreadyEscaped bool
 }
 
 func (e LiteralExpr) Accept(v ExprVisitor) interface{} {
 	return v.VisitLiteral(e)
 }
 
+type TupleExpr struct {
+	Exprs []Expr
+}
+
+func NewTupleExpr(exprs ...Expr) TupleExpr {
+	return TupleExpr{Exprs: exprs}
+}
+
+func (e TupleExpr) Accept(v ExprVisitor) interface{} {
+	return v.VisitTuple(e)
+}
+
 type InfixExpr struct {
 	Left  Expr
 	Op    string
@@ -112,16 +121,10 @@ func NewCountFunc(args ...Expr) FunctionExpr {
 
 var NewWildcardExpr = LiteralExpr{Value: "*"}
 
-// NewLiteral can be used in 95% of cases, where we create a simple new Literal.
-// Use NewLiteralWithEscape if you copy another LiteralExpr which can have LiteralAlreadyEscaped set to true.
 func NewLiteral(value any) LiteralExpr {
 	return LiteralExpr{Value: value}
 }
 
-func NewLiteralWithEscape(value any, literalAlreadyEscaped bool) LiteralExpr {
-	return LiteralExpr{Value: value, LiteralAlreadyEscaped: literalAlreadyEscaped}
-}
-
 // DistinctExpr is a representation of DISTINCT keyword in SQL, e.g. `SELECT DISTINCT` ... or `SELECT COUNT(DISTINCT ...)`
 type DistinctExpr struct {
 	Expr Expr
@@ -287,6 +290,7 @@ func (e CTE) Accept(v ExprVisitor) interface{} {
 type ExprVisitor interface {
 	VisitFunction(e FunctionExpr) interface{}
 	VisitLiteral(l LiteralExpr) interface{}
+	VisitTuple(e TupleExpr) interface{}
 	VisitInfix(e InfixExpr) interface{}
 	VisitColumnRef(e ColumnRef) interface{}
 	VisitPrefixExpr(e PrefixExpr) interface{}

quesma/model/expr_string_renderer.go

@@ -4,6 +4,7 @@ package model
 
 import (
 	"fmt"
+	"quesma/logger"
 	"quesma/quesma/types"
 	"quesma/util"
 	"regexp"
@@ -65,19 +66,26 @@ func (v *renderer) VisitFunction(e FunctionExpr) interface{} {
 	return e.Name + "(" + strings.Join(args, ",") + ")"
 }
 
-// VisitLiteral escapes string values, as e.g. ' can come inside string in a query,
-// but Clickhouse requires it to be escaped as \' or ”
-// See more https://clickhouse.com/docs/en/sql-reference/syntax > String
-// Also tested "\n" and "\t" in the strings, and the way we render them works for Clickhouse.
-// Not 100% sure about the other special characters (e.g. \r and \b), but they shouldn't be used very often.
 func (v *renderer) VisitLiteral(l LiteralExpr) interface{} {
-	valueStr, isStr := l.Value.(string)
-	if isStr && !l.LiteralAlreadyEscaped {
-		return EscapeString(valueStr)
-	}
 	return fmt.Sprintf("%v", l.Value)
 }
 
+func (v *renderer) VisitTuple(t TupleExpr) interface{} {
+	switch len(t.Exprs) {
+	case 0:
+		logger.WarnWithThrottling("VisitTuple", "TupleExpr with no expressions")
+		return "()"
+	case 1:
+		return t.Exprs[0].Accept(v)
+	default:
+		args := make([]string, len(t.Exprs))
+		for i, arg := range t.Exprs {
+			args[i] = arg.Accept(v).(string)
+		}
+		return fmt.Sprintf("(%s)", strings.Join(args, ","))
+	}
+}
+
 func (v *renderer) VisitInfix(e InfixExpr) interface{} {
 	var lhs, rhs interface{} // TODO FOR NOW LITTLE PARANOID BUT HELPS ME NOT SEE MANY PANICS WHEN TESTING
 	if e.Left != nil {

quesma/queryparser/query_parser.go

@@ -506,20 +506,12 @@ func (cw *ClickhouseQueryTranslator) parseTerms(queryMap QueryMap) model.SimpleQ
 			simpleStatement := model.NewInfixExpr(model.NewColumnRef(k), "=", model.NewLiteral(sprint(vAsArray[0])))
 			return model.NewSimpleQuery(simpleStatement, true)
 		}
-		values := make([]string, len(vAsArray))
+		values := make([]model.Expr, len(vAsArray))
 		for i, v := range vAsArray {
-			if asStr, isStr := v.(string); isStr {
-				// Special case, only one so far, where we have to escape the string here, not in the renderer.
-				// Normally we escape (change e.g. ' -> \'), but here we want to have those ' unescaped,
-				// e.g. "IN ('abc', 'def')", without escaping those 4 '.
-				values[i] = util.SingleQuote(model.EscapeString(asStr))
-			} else {
-				values[i] = sprint(v)
-			}
+			values[i] = model.NewLiteral(sprint(v))
 		}
-		combinedValues := model.NewLiteral("(" + strings.Join(values, ",") + ")")
-		combinedValues.LiteralAlreadyEscaped = true
-		compoundStatement := model.NewInfixExpr(model.NewColumnRef(k), "IN", combinedValues)
+		tuple := model.NewTupleExpr(values...)
+		compoundStatement := model.NewInfixExpr(model.NewColumnRef(k), "IN", tuple)
 		return model.NewSimpleQuery(compoundStatement, true)
 	}
 

quesma/quesma/schema_transformer.go

@@ -35,10 +35,10 @@ func (s *SchemaCheckPass) applyBooleanLiteralLowering(index schema.Schema, query
 			if strings.Contains(boolLiteral, "true") || strings.Contains(boolLiteral, "false") {
 				boolLiteral = strings.TrimLeft(boolLiteral, "'")
 				boolLiteral = strings.TrimRight(boolLiteral, "'")
-				return model.NewLiteralWithEscape(boolLiteral, e.LiteralAlreadyEscaped)
+				return model.NewLiteral(boolLiteral)
 			}
 		}
-		return model.NewLiteralWithEscape(e.Value, e.LiteralAlreadyEscaped)
+		return model.NewLiteral(e.Value)
 	}
 
 	expr := query.SelectCommand.Accept(visitor)
@@ -973,9 +973,9 @@ func (s *SchemaCheckPass) applyMatchOperator(indexSchema schema.Schema, query *m
 
 			switch field.Type.String() {
 			case schema.QuesmaTypeInteger.Name, schema.QuesmaTypeLong.Name, schema.QuesmaTypeUnsignedLong.Name, schema.QuesmaTypeBoolean.Name:
-				return model.NewInfixExpr(lhs, "=", model.NewLiteralWithEscape(rhsValue, rhs.LiteralAlreadyEscaped))
+				return model.NewInfixExpr(lhs, "=", model.NewLiteral(rhsValue))
 			default:
-				return model.NewInfixExpr(lhs, "iLIKE", model.NewLiteralWithEscape("'%"+rhsValue+"%'", rhs.LiteralAlreadyEscaped))
+				return model.NewInfixExpr(lhs, "iLIKE", model.NewLiteral("'%"+rhsValue+"%'"))
 			}
 		}
 

quesma/testdata/requests.go

@@ -1006,7 +1006,7 @@ var TestsSearch = []SearchTestCase{
 						},
 						{
 							"terms": {
-								"task.enabled": [true, 54]
+								"task.enabled": [true, 54, "abc", "abc's"]
 							}
 						}
 					]