revert part of the old COALESCE handling to support missing joined relation fields comparison with empty string

2023-04-03 20:27:52 +03:00
parent 850fe353da
commit f31a3b133c
4 changed files with 118 additions and 28 deletions
@@ -107,15 +107,9 @@ func buildExpr(

 	switch op {
 	case fexpr.SignEq, fexpr.SignAnyEq:
-		expr = dbx.NewExp(
-			fmt.Sprintf("%s = %s", normalizeNullIdentifier(left), normalizeNullIdentifier(right)),
-			mergeParams(left.Params, right.Params),
-		)
+		expr = resolveEqualExpr(true, left, right)
 	case fexpr.SignNeq, fexpr.SignAnyNeq:
-		expr = dbx.NewExp(
-			fmt.Sprintf("%s != %s", normalizeNullIdentifier(left), normalizeNullIdentifier(right)),
-			mergeParams(left.Params, right.Params),
-		)
+		expr = resolveEqualExpr(false, left, right)
 	case fexpr.SignLike, fexpr.SignAnyLike:
 		// the right side is a column and therefor wrap it with "%" for contains like behavior
 		if len(right.Params) == 0 {
@@ -238,18 +232,108 @@ func resolveToken(token fexpr.Token, fieldResolver FieldResolver) (*ResolverResu
 	return nil, errors.New("unresolvable token type")
 }

-func normalizeNullIdentifier(result *ResolverResult) string {
-	lower := strings.ToLower(result.Identifier)
+// Resolves = and != expressions in an attempt to minimize the COALESCE
+// usage and to gracefully handle null vs empty string normalizations.
+//
+// The expression `a = "" OR a is null` tends to perform better than
+// `COALESCE(a, "") = ""` since the direct match can be accomplished
+// with a seek while the COALESCE will induce a table scan.
+func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
+	isLeftEmpty := isEmptyIdentifier(left) || (len(left.Params) == 1 && hasEmptyParamValue(left))
+	isRightEmpty := isEmptyIdentifier(right) || (len(right.Params) == 1 && hasEmptyParamValue(right))

-	if lower == "null" {
-		return "''"
+	sign := "="
+	nullExpr := "IS NULL"
+	concatOp := "OR"
+	if !equal {
+		sign = "!="
+		nullExpr = "IS NOT NULL"
+		concatOp = "AND"
 	}

-	if strings.Contains(lower, "json_extract(") || strings.Contains(lower, "json_array_length(") {
-		return fmt.Sprintf("COALESCE(%s, '')", result.Identifier)
+	// both operands are empty
+	if isLeftEmpty && isRightEmpty {
+		return dbx.NewExp(fmt.Sprintf("'' %s ''", sign), mergeParams(left.Params, right.Params))
 	}

-	return result.Identifier
+	// direct compare since at least one of the operands is known to be non-empty
+	// eg. a = 'example'
+	if isKnownNonEmptyIdentifier(left) || isKnownNonEmptyIdentifier(right) {
+		leftIdentifier := left.Identifier
+		if isLeftEmpty {
+			leftIdentifier = "''"
+		}
+		rightIdentifier := right.Identifier
+		if isRightEmpty {
+			rightIdentifier = "''"
+		}
+		return dbx.NewExp(
+			fmt.Sprintf("%s %s %s", leftIdentifier, sign, rightIdentifier),
+			mergeParams(left.Params, right.Params),
+		)
+	}
+
+	// "" = b OR b IS NULL
+	// "" != b AND b IS NOT NULL
+	if isLeftEmpty {
+		return dbx.NewExp(
+			fmt.Sprintf("('' %s %s %s %s %s)", sign, right.Identifier, concatOp, right.Identifier, nullExpr),
+			mergeParams(left.Params, right.Params),
+		)
+	}
+
+	// a = "" OR a IS NULL
+	// a != "" AND a IS NOT NULL
+	if isRightEmpty {
+		return dbx.NewExp(
+			fmt.Sprintf("(%s %s '' %s %s %s)", left.Identifier, sign, concatOp, left.Identifier, nullExpr),
+			mergeParams(left.Params, right.Params),
+		)
+	}
+
+	// fallback to a COALESCE comparison
+	return dbx.NewExp(
+		fmt.Sprintf(
+			"COALESCE(%s, '') %s COALESCE(%s, '')",
+			left.Identifier,
+			sign,
+			right.Identifier,
+		),
+		mergeParams(left.Params, right.Params),
+	)
+}
+
+func hasEmptyParamValue(result *ResolverResult) bool {
+	for _, p := range result.Params {
+		switch v := p.(type) {
+		case nil:
+			return true
+		case string:
+			if v == "" {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
+func isKnownNonEmptyIdentifier(result *ResolverResult) bool {
+	switch strings.ToLower(result.Identifier) {
+	case "1", "0", "false", `true`:
+		return true
+	}
+
+	return len(result.Params) > 0 && !hasEmptyParamValue(result) && !isEmptyIdentifier(result)
+}
+
+func isEmptyIdentifier(result *ResolverResult) bool {
+	switch strings.ToLower(result.Identifier) {
+	case "", "null", "''", `""`, "``":
+		return true
+	default:
+		return false
+	}
 }

 func isAnyMatchOp(op fexpr.SignOp) bool {
@@ -48,6 +48,12 @@ func TestFilterDataBuildExpr(t *testing.T) {
 				regexp.QuoteMeta("}") +
 				"$",
 		},
+		{
+			"empty string vs null",
+			"'' = null && null != ''",
+			false,
+			"('' = '' AND '' != '')",
+		},
 		{
 			"like with 2 columns",
 			"test1 ~ test2",
@@ -125,21 +131,21 @@ func TestFilterDataBuildExpr(t *testing.T) {
 				".+" +
 				regexp.QuoteMeta("}) AND [[test3]] LIKE {:") +
 				".+" +
-				regexp.QuoteMeta("} ESCAPE '\\' AND [[test4.sub]] = '')") +
+				regexp.QuoteMeta("} ESCAPE '\\' AND ([[test4.sub]] = '' OR [[test4.sub]] IS NULL))") +
 				"$",
 		},
 		{
 			"combination of special literals (null, true, false)",
-			"test1=true && test2 != false && test3 = null || test4.sub != null",
+			"test1=true && test2 != false && null = test3 || null != test4.sub",
 			false,
-			"^" + regexp.QuoteMeta("([[test1]] = 1 AND [[test2]] != 0 AND [[test3]] = '' OR [[test4.sub]] != '')") + "$",
+			"^" + regexp.QuoteMeta("([[test1]] = 1 AND [[test2]] != 0 AND ('' = [[test3]] OR [[test3]] IS NULL) OR ('' != [[test4.sub]] AND [[test4.sub]] IS NOT NULL))") + "$",
 		},
 		{
 			"all operators",
 			"(test1 = test2 || test2 != test3) && (test2 ~ 'example' || test2 !~ '%%abc') && 'switch1%%' ~ test1 && 'switch2' !~ test2 && test3 > 1 && test3 >= 0 && test3 <= 4 && 2 < 5",
 			false,
 			"^" +
-				regexp.QuoteMeta("(([[test1]] = [[test2]] OR [[test2]] != [[test3]]) AND ([[test2]] LIKE {:") +
+				regexp.QuoteMeta("((COALESCE([[test1]], '') = COALESCE([[test2]], '') OR COALESCE([[test2]], '') != COALESCE([[test3]], '')) AND ([[test2]] LIKE {:") +
 				".+" +
 				regexp.QuoteMeta("} ESCAPE '\\' OR [[test2]] NOT LIKE {:") +
 				".+" +
@@ -274,8 +274,8 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
 			false,
 			`{"page":1,"perPage":` + fmt.Sprint(MaxPerPage) + `,"totalItems":1,"totalPages":1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`,
 			[]string{
-				"SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (test2 != '')) AND (test1 >= 2)",
-				"SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (test2 != '')) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT 500",
+				"SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2)",
+				"SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT 500",
 			},
 		},
 		{
@@ -287,8 +287,8 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
 			false,
 			`{"page":1,"perPage":10,"totalItems":0,"totalPages":0,"items":[]}`,
 			[]string{
-				"SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (test3 != '')",
-				"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (test3 != '') ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
+				"SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL)))",
+				"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL))) ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
 			},
 		},
 		{