normalized null handling in search filters

Bufkin · Jul 18, 2022 · 47fc9b1 · 47fc9b1
1 parent eaf08a5
commit 47fc9b1
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 26 deletions.
diff --git a/daos/record.go b/daos/record.go
@@ -331,15 +331,15 @@ func (dao *Dao) SyncRecordTableSchema(newCollection *models.Collection, oldColle
 		// check for new or renamed columns
 		for _, field := range newSchema.Fields() {
 			oldField := oldSchema.GetFieldById(field.Id)
-			if oldField != nil {
-				// rename
-				_, err := txDao.DB().RenameColumn(newTableName, oldField.Name, field.Name).Execute()
+			if oldField == nil {
+				// add
+				_, err := txDao.DB().AddColumn(newTableName, field.Name, field.ColDefinition()).Execute()
 				if err != nil {
 					return err
 				}
-			} else {
-				// add
-				_, err := txDao.DB().AddColumn(newTableName, field.Name, field.ColDefinition()).Execute()
+			} else if oldField.Name != field.Name {
+				// rename
+				_, err := txDao.DB().RenameColumn(newTableName, oldField.Name, field.Name).Execute()
 				if err != nil {
 					return err
 				}

diff --git a/tools/search/filter.go b/tools/search/filter.go
@@ -98,17 +98,9 @@ func (f FilterData) resolveTokenizedExpr(expr fexpr.Expr, fieldResolver FieldRes
 
 	switch expr.Op {
 	case fexpr.SignEq:
-		op := "="
-		if strings.ToLower(lName) == "null" || strings.ToLower(rName) == "null" {
-			op = "IS"
-		}
-		return dbx.NewExp(fmt.Sprintf("%s %s %s", lName, op, rName), params), nil
+		return dbx.NewExp(fmt.Sprintf("COALESCE(%s, '') = COALESCE(%s, '')", lName, rName), params), nil
 	case fexpr.SignNeq:
-		op := "!="
-		if strings.ToLower(lName) == "null" || strings.ToLower(rName) == "null" {
-			op = "IS NOT"
-		}
-		return dbx.NewExp(fmt.Sprintf("%s %s %s", lName, op, rName), params), nil
+		return dbx.NewExp(fmt.Sprintf("COALESCE(%s, '') != COALESCE(%s, '')", lName, rName), params), nil
 	case fexpr.SignLike:
 		// normalize operands and switch sides if the left operand is a number or text
 		if len(lParams) > 0 {

diff --git a/tools/search/filter_test.go b/tools/search/filter_test.go
@@ -39,25 +39,25 @@ func TestFilterDataBuildExpr(t *testing.T) {
 			"^" +
 				regexp.QuoteMeta("((([[test1]] > {:") +
 				".+" +
-				regexp.QuoteMeta("}) OR ([[test2]] != {:") +
+				regexp.QuoteMeta("}) OR (COALESCE([[test2]], '') != COALESCE({:") +
 				".+" +
-				regexp.QuoteMeta("})) AND ([[test3]] LIKE {:") +
+				regexp.QuoteMeta("}, ''))) AND ([[test3]] LIKE {:") +
 				".+" +
-				regexp.QuoteMeta("})) AND ([[test4.sub]] IS NULL)") +
+				regexp.QuoteMeta("})) AND (COALESCE([[test4.sub]], '') = COALESCE(NULL, ''))") +
 				"$",
 		},
 		// combination of special literals (null, true, false)
 		{
 			"test1=true && test2 != false && test3 = null || test4.sub != null",
 			false,
-			"^" + regexp.QuoteMeta("((([[test1]] = 1) AND ([[test2]] != 0)) AND ([[test3]] IS NULL)) OR ([[test4.sub]] IS NOT NULL)") + "$",
+			"^" + regexp.QuoteMeta("(((COALESCE([[test1]], '') = COALESCE(1, '')) AND (COALESCE([[test2]], '') != COALESCE(0, ''))) AND (COALESCE([[test3]], '') = COALESCE(NULL, ''))) OR (COALESCE([[test4.sub]], '') != COALESCE(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("}) OR ([[test2]] NOT LIKE {:") +
 				".+" +

diff --git a/tools/search/provider_test.go b/tools/search/provider_test.go
@@ -273,8 +273,8 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
 			false,
 			`{"page":1,"perPage":` + fmt.Sprint(MaxPerPage) + `,"totalItems":1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`,
 			[]string{
-				"SELECT count(*) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (test2 IS NOT null)) AND (test1 >= '2') ORDER BY `test1` ASC, `test2` DESC",
-				"SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (test2 IS NOT null)) AND (test1 >= '2') ORDER BY `test1` ASC, `test2` DESC LIMIT 200",
+				"SELECT count(*) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (COALESCE(test2, '') != COALESCE(null, ''))) AND (test1 >= '2') ORDER BY `test1` ASC, `test2` DESC",
+				"SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (COALESCE(test2, '') != COALESCE(null, ''))) AND (test1 >= '2') ORDER BY `test1` ASC, `test2` DESC LIMIT 200",
 			},
 		},
 		// valid sort and filter fields (zero results)
@@ -286,8 +286,8 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
 			false,
 			`{"page":1,"perPage":10,"totalItems":0,"items":[]}`,
 			[]string{
-				"SELECT count(*) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (test3 != '') ORDER BY `test1` ASC, `test3` ASC",
-				"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (test3 != '') ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
+				"SELECT count(*) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (COALESCE(test3, '') != COALESCE('', '')) ORDER BY `test1` ASC, `test3` ASC",
+				"SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (COALESCE(test3, '') != COALESCE('', '')) ORDER BY `test1` ASC, `test3` ASC LIMIT 10",
 			},
 		},
 		// pagination test
@@ -344,7 +344,7 @@ func TestProviderExecNonEmptyQuery(t *testing.T) {
 
 		for _, q := range testDB.CalledQueries {
 			if !list.ExistInSliceWithRegex(q, s.expectQueries) {
-				t.Errorf("(%d) Didn't expect query %v", i, q)
+				t.Errorf("(%d) Didn't expect query \n%v", i, q)
 			}
 		}
 	}