fix nullable non-equal comparisions

This commit is contained in:
Gani Georgiev
2023-12-08 13:50:12 +02:00
parent 1bf7f148b0
commit 4d3ba270c0
6 changed files with 94 additions and 85 deletions
+6 -3
View File
@@ -293,7 +293,10 @@ func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
concatOp := "OR"
nullExpr := "IS NULL"
if !equal {
equalOp = "!="
// use `IS NOT` instead of `!=` because direct non-equal comparisions
// to nullable column values that are actually NULL yields to NULL instead of TRUE, eg.:
// `'example' != nullableColumn` -> NULL even if nullableColumn row value is NULL
equalOp = "IS NOT"
concatOp = "AND"
nullExpr = "IS NOT NULL"
}
@@ -321,7 +324,7 @@ func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
}
// "" = b OR b IS NULL
// "" != b AND b IS NOT NULL
// "" IS NOT b AND b IS NOT NULL
if isLeftEmpty {
return dbx.NewExp(
fmt.Sprintf("('' %s %s %s %s %s)", equalOp, right.Identifier, concatOp, right.Identifier, nullExpr),
@@ -330,7 +333,7 @@ func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
}
// a = "" OR a IS NULL
// a != "" AND a IS NOT NULL
// a IS NOT "" AND a IS NOT NULL
if isRightEmpty {
return dbx.NewExp(
fmt.Sprintf("(%s %s '' %s %s %s)", left.Identifier, equalOp, concatOp, left.Identifier, nullExpr),
+4 -4
View File
@@ -55,7 +55,7 @@ func TestFilterDataBuildExpr(t *testing.T) {
"empty string vs null",
"'' = null && null != ''",
false,
"('' = '' AND '' != '')",
"('' = '' AND '' IS NOT '')",
},
{
"like with 2 columns",
@@ -118,19 +118,19 @@ func TestFilterDataBuildExpr(t *testing.T) {
"complex expression",
"((test1 > 1) || (test2 != 2)) && test3 ~ '%%example' && test4_sub = null",
false,
"(([[test1]] > {:TEST} OR [[test2]] != {:TEST}) AND [[test3]] LIKE {:TEST} ESCAPE '\\' AND ([[test4_sub]] = '' OR [[test4_sub]] IS NULL))",
"(([[test1]] > {:TEST} OR [[test2]] IS NOT {:TEST}) AND [[test3]] LIKE {:TEST} ESCAPE '\\' AND ([[test4_sub]] = '' OR [[test4_sub]] IS NULL))",
},
{
"combination of special literals (null, true, false)",
"test1=true && test2 != false && null = test3 || null != test4_sub",
false,
"([[test1]] = 1 AND [[test2]] != 0 AND ('' = [[test3]] OR [[test3]] IS NULL) OR ('' != [[test4_sub]] AND [[test4_sub]] IS NOT NULL))",
"([[test1]] = 1 AND [[test2]] IS NOT 0 AND ('' = [[test3]] OR [[test3]] IS NULL) OR ('' IS NOT [[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,
"((COALESCE([[test1]], '') = COALESCE([[test2]], '') OR COALESCE([[test2]], '') != COALESCE([[test3]], '')) AND ([[test2]] LIKE {:TEST} ESCAPE '\\' OR [[test2]] NOT LIKE {:TEST} ESCAPE '\\') AND {:TEST} LIKE ('%' || [[test1]] || '%') ESCAPE '\\' AND {:TEST} NOT LIKE ('%' || [[test2]] || '%') ESCAPE '\\' AND [[test3]] > {:TEST} AND [[test3]] >= {:TEST} AND [[test3]] <= {:TEST} AND {:TEST} < {:TEST})",
"((COALESCE([[test1]], '') = COALESCE([[test2]], '') OR COALESCE([[test2]], '') IS NOT COALESCE([[test3]], '')) AND ([[test2]] LIKE {:TEST} ESCAPE '\\' OR [[test2]] NOT LIKE {:TEST} ESCAPE '\\') AND {:TEST} LIKE ('%' || [[test1]] || '%') ESCAPE '\\' AND {:TEST} NOT LIKE ('%' || [[test2]] || '%') ESCAPE '\\' AND [[test3]] > {:TEST} AND [[test3]] >= {:TEST} AND [[test3]] <= {:TEST} AND {:TEST} < {:TEST})",
},
}
+44 -44
View File
@@ -308,8 +308,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 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 " + fmt.Sprint(MaxPerPage),
"SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 IS NOT '' AND test2 IS NOT NULL)))) AND (test1 >= 2)",
"SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 IS NOT '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT " + fmt.Sprint(MaxPerPage),
},
},
{
@@ -322,7 +322,7 @@ 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 * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT " + fmt.Sprint(MaxPerPage),
"SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 IS NOT '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT " + fmt.Sprint(MaxPerPage),
},
},
{
@@ -335,8 +335,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 != '' 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",
"SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 IS NOT '' AND test3 IS NOT NULL)))",
"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 IS NOT '' AND test3 IS NOT NULL))) ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
},
},
{
@@ -349,7 +349,7 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
false,
`{"page":1,"perPage":10,"totalItems":-1,"totalPages":-1,"items":[]}`,
[]string{
"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL))) ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 IS NOT '' AND test3 IS NOT NULL))) ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
},
},
{
@@ -382,48 +382,48 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
}
for _, s := range scenarios {
testDB.CalledQueries = []string{} // reset
t.Run(s.name, func(t *testing.T) {
testDB.CalledQueries = []string{} // reset
testResolver := &testFieldResolver{}
p := NewProvider(testResolver).
Query(query).
Page(s.page).
PerPage(s.perPage).
Sort(s.sort).
SkipTotal(s.skipTotal).
Filter(s.filter)
testResolver := &testFieldResolver{}
p := NewProvider(testResolver).
Query(query).
Page(s.page).
PerPage(s.perPage).
Sort(s.sort).
SkipTotal(s.skipTotal).
Filter(s.filter)
result, err := p.Exec(&[]testTableStruct{})
result, err := p.Exec(&[]testTableStruct{})
hasErr := err != nil
if hasErr != s.expectError {
t.Errorf("[%s] Expected hasErr %v, got %v (%v)", s.name, s.expectError, hasErr, err)
continue
}
if hasErr {
continue
}
if testResolver.UpdateQueryCalls != 1 {
t.Errorf("[%s] Expected resolver.Update to be called %d, got %d", s.name, 1, testResolver.UpdateQueryCalls)
}
encoded, _ := json.Marshal(result)
if string(encoded) != s.expectResult {
t.Errorf("[%s] Expected result %v, got \n%v", s.name, s.expectResult, string(encoded))
}
if len(s.expectQueries) != len(testDB.CalledQueries) {
t.Errorf("[%s] Expected %d queries, got %d: \n%v", s.name, len(s.expectQueries), len(testDB.CalledQueries), testDB.CalledQueries)
continue
}
for _, q := range testDB.CalledQueries {
if !list.ExistInSliceWithRegex(q, s.expectQueries) {
t.Fatalf("[%s] Didn't expect query \n%v \nin \n%v", s.name, q, s.expectQueries)
hasErr := err != nil
if hasErr != s.expectError {
t.Fatalf("Expected hasErr %v, got %v (%v)", s.expectError, hasErr, err)
}
}
if hasErr {
return
}
if testResolver.UpdateQueryCalls != 1 {
t.Fatalf("Expected resolver.Update to be called %d, got %d", 1, testResolver.UpdateQueryCalls)
}
encoded, _ := json.Marshal(result)
if string(encoded) != s.expectResult {
t.Fatalf("Expected result %v, got \n%v", s.expectResult, string(encoded))
}
if len(s.expectQueries) != len(testDB.CalledQueries) {
t.Fatalf("Expected %d queries, got %d: \n%v", len(s.expectQueries), len(testDB.CalledQueries), testDB.CalledQueries)
}
for _, q := range testDB.CalledQueries {
if !list.ExistInSliceWithRegex(q, s.expectQueries) {
t.Fatalf("Didn't expect query \n%v \nin \n%v", q, s.expectQueries)
}
}
})
}
}