Fix issue with searching for metric names via regex (#418)

* Remove unnecessary call to fmt.Sprintf * Use uppercase for SQL expression * Fix linting issues * Ensure null values are converted to empty strings in SQLite
G-Research · Oct 3, 2023 · 0f200ab · 0f200ab
1 parent 7fe13f1
commit 0f200ab
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 11 deletions.
diff --git a/pkg/api/aim/query/clause.go b/pkg/api/aim/query/clause.go
@@ -1,9 +1,8 @@
 package query
 
 import (
-	"fmt"
-
 	"gorm.io/driver/postgres"
+	"gorm.io/driver/sqlite"
 	"gorm.io/gorm/clause"
 )
 
@@ -15,24 +14,41 @@ type Regexp struct {
 
 // Build builds positive statement.
 func (regexp Regexp) Build(builder clause.Builder) {
-	builder.WriteQuoted(regexp.Column)
+	regexp.writeColumn(builder)
 	switch regexp.Dialector {
 	case postgres.Dialector{}.Name():
+		// #nosec G104
 		builder.WriteString(" ~ ")
 	default:
-		builder.WriteString(" regexp ")
+		// #nosec G104
+		builder.WriteString(" REGEXP ")
 	}
-	builder.AddVar(builder, fmt.Sprintf("%s", regexp.Value))
+	builder.AddVar(builder, regexp.Value)
 }
 
 // NegationBuild builds negative statement.
 func (regexp Regexp) NegationBuild(builder clause.Builder) {
-	builder.WriteQuoted(regexp.Column)
+	regexp.writeColumn(builder)
 	switch regexp.Dialector {
 	case postgres.Dialector{}.Name():
+		// #nosec G104
 		builder.WriteString(" !~ ")
 	default:
-		builder.WriteString(" NOT regexp ")
+		// #nosec G104
+		builder.WriteString(" NOT REGEXP ")
 	}
 	builder.AddVar(builder, regexp.Value)
 }
+
+func (regexp Regexp) writeColumn(builder clause.Builder) {
+	switch regexp.Dialector {
+	case sqlite.Dialector{}.Name():
+		// #nosec G104
+		builder.WriteString("IFNULL(")
+		builder.WriteQuoted(regexp.Column)
+		// #nosec G104
+		builder.WriteString(", '')")
+	default:
+		builder.WriteQuoted(regexp.Column)
+	}
+}
diff --git a/pkg/api/aim/query/query_test.go b/pkg/api/aim/query/query_test.go
@@ -193,28 +193,28 @@ func (s *QueryTestSuite) TestSqliteDialector_Ok() {
 			name:  "TestRunNameWithRegexpMatchFunction",
 			query: `(re.match('run', run.name))`,
 			expectedSQL: `SELECT "run_uuid" FROM "runs" ` +
-				`WHERE ("runs"."name" regexp $1 AND "runs"."lifecycle_stage" <> $2)`,
+				`WHERE (IFNULL("runs"."name", '') REGEXP $1 AND "runs"."lifecycle_stage" <> $2)`,
 			expectedVars: []interface{}{"^run", models.LifecycleStageDeleted},
 		},
 		{
 			name:  "TestRunNameWithRegexpSearchFunction",
 			query: `(re.search('run', run.name))`,
 			expectedSQL: `SELECT "run_uuid" FROM "runs" ` +
-				`WHERE ("runs"."name" regexp $1 AND "runs"."lifecycle_stage" <> $2)`,
+				`WHERE (IFNULL("runs"."name", '') REGEXP $1 AND "runs"."lifecycle_stage" <> $2)`,
 			expectedVars: []interface{}{"run", models.LifecycleStageDeleted},
 		},
 		{
 			name:  "TestRunNameWithNegatedRegexpMatchFunction",
 			query: `not (re.match('run', run.name))`,
 			expectedSQL: `SELECT "run_uuid" FROM "runs" ` +
-				`WHERE ("runs"."name" NOT regexp $1 AND "runs"."lifecycle_stage" <> $2)`,
+				`WHERE (IFNULL("runs"."name", '') NOT REGEXP $1 AND "runs"."lifecycle_stage" <> $2)`,
 			expectedVars: []interface{}{"^run", models.LifecycleStageDeleted},
 		},
 		{
 			name:  "TestRunNameWithNegatedRegexpSearchFunction",
 			query: `not (re.search('run', run.name))`,
 			expectedSQL: `SELECT "run_uuid" FROM "runs" ` +
-				`WHERE ("runs"."name" NOT regexp $1 AND "runs"."lifecycle_stage" <> $2)`,
+				`WHERE (IFNULL("runs"."name", '') NOT REGEXP $1 AND "runs"."lifecycle_stage" <> $2)`,
 			expectedVars: []interface{}{"run", models.LifecycleStageDeleted},
 		},
 		{