Merge pull request #2 from gabisonia/integration-reliability-and-filter-safety

Integration reliability and filter safety

Author: gabisonia

.github/workflows/release.yml

@@ -7,7 +7,7 @@ on:
   workflow_dispatch:
     inputs:
       version:
-        description: "Semver version (e.g. 0.2.0 or v0.2.0)"
+        description: "Semver version (e.g. 0.2.1 or v0.2.1)"
         required: true
         type: string
 

Makefile

@@ -0,0 +1,23 @@
+.PHONY: test-integration-all test-integration-stores test-integration-postgres test-integration-mssql test-integration-msql
+
+GO ?= go
+GOTOOLCHAIN ?= local
+INTEGRATION_TAG ?= integration
+MOD_MODE ?= mod
+TEST_TIMEOUT ?= 30m
+TEST_FLAGS ?=
+GO_ENV ?= env -u GOROOT GOTOOLCHAIN=$(GOTOOLCHAIN)
+
+test-integration-all:
+	$(GO_ENV) $(GO) test -mod=$(MOD_MODE) -tags=$(INTEGRATION_TAG) -timeout=$(TEST_TIMEOUT) $(TEST_FLAGS) ./...
+
+test-integration-stores:
+	$(GO_ENV) $(GO) test -mod=$(MOD_MODE) -tags=$(INTEGRATION_TAG) -timeout=$(TEST_TIMEOUT) $(TEST_FLAGS) ./stores/...
+
+test-integration-postgres:
+	$(GO_ENV) $(GO) test -mod=$(MOD_MODE) -tags=$(INTEGRATION_TAG) -timeout=$(TEST_TIMEOUT) $(TEST_FLAGS) ./stores/postgres
+
+test-integration-mssql:
+	$(GO_ENV) $(GO) test -mod=$(MOD_MODE) -tags=$(INTEGRATION_TAG) -timeout=$(TEST_TIMEOUT) $(TEST_FLAGS) ./stores/mssql
+
+test-integration-msql: test-integration-mssql

README.md

@@ -229,12 +229,12 @@ GitHub Actions workflows are configured for:
 
 Release options:
 
-1. Manual (recommended): run `Release` workflow via GitHub UI with `version` input (`0.2.0` or `v0.2.0`).
+1. Manual (recommended): run `Release` workflow via GitHub UI with `version` input (`0.2.1` or `v0.2.1`).
 2. Tag-driven: push a semver tag and the workflow publishes release notes automatically:
 
 ```bash
-git tag v0.2.0
-git push origin v0.2.0
+git tag v0.2.1
+git push origin v0.2.1
 ```
 
 ## Samples

go.mod

@@ -28,6 +28,8 @@ require (
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-ole/go-ole v1.2.6 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 // indirect
+	github.com/golang-sql/sqlexp v0.1.0 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/jackc/pgpassfile v1.0.0 // indirect
 	github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect

go.sum

@@ -36,10 +36,10 @@ github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY=
 github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
-github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 h1:C/Fq8+Vm3x8+3xB3SO2/3A8Q+7WmfYI8MViMbum/v8A=
-github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9/go.mod h1:9x5EikdFyA8Kj4m2Vj93QShv2N6Yf4H6J5D9Q1QzM4Y=
-github.com/golang-sql/sqlexp v0.1.0 h1:DKM0nqnvWFfB6K6x2zI2Qf8F9Pwc7P0W8o2JQ7QY9MM=
-github.com/golang-sql/sqlexp v0.1.0/go.mod h1:hxXw1K3Zk7K8Y6nG5N7f6d4rV1E8j9a7q6x5y4z3w2Q=
+github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 h1:au07oEsX2xN0ktxqI+Sida1w446QrXBRJ0nee3SNZlA=
+github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0=
+github.com/golang-sql/sqlexp v0.1.0 h1:ZCD6MBpcuOVfGVqsEmY5/4FtYiKz6tSyUv9LPEDei6A=
+github.com/golang-sql/sqlexp v0.1.0/go.mod h1:J4ad9Vo8ZCWQ2GMrC4UCQy1JpCbwU9m3EOqtpKwwwHI=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=

stores/mssql/mssql_integration_test.go

@@ -33,7 +33,7 @@ var (
 )
 
 func TestMain(m *testing.M) {
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	ctx, cancel := context.WithTimeout(context.Background(), 12*time.Minute)
 	defer cancel()
 
 	dsn := strings.TrimSpace(os.Getenv("MSSQL_TEST_DSN"))
@@ -67,15 +67,17 @@ func TestMain(m *testing.M) {
 
 func startMSSQLContainer(ctx context.Context) (testcontainers.Container, string, error) {
 	request := testcontainers.ContainerRequest{
-		Image:        "mcr.microsoft.com/mssql/server:2022-latest",
-		ExposedPorts: []string{"1433/tcp"},
+		Image:           "mcr.microsoft.com/mssql/server:2022-latest",
+		ImagePlatform:   "linux/amd64",
+		AlwaysPullImage: true,
+		ExposedPorts:    []string{"1433/tcp"},
 		Env: map[string]string{
 			"ACCEPT_EULA":       "Y",
 			"MSSQL_SA_PASSWORD": integrationMSSQLPassword,
 			"MSSQL_PID":         "Developer",
 		},
 		WaitingFor: wait.ForLog("SQL Server is now ready for client connections").
-			WithStartupTimeout(4 * time.Minute),
+			WithStartupTimeout(10 * time.Minute),
 	}
 
 	container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{

vectordata/filter_sql.go

@@ -6,6 +6,8 @@ import (
 	"strings"
 )
 
+const numericTextPattern = `^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)([eE][+-]?[0-9]+)?$`
+
 // FilterSQLConfig configures filter compilation into SQL expressions.
 type FilterSQLConfig struct {
 	// ColumnExpr maps logical column names to pre-quoted SQL expressions.
@@ -123,7 +125,7 @@ func (c *filterCompiler) compileGt(node GtFilter) (string, error) {
 		return fmt.Sprintf("(%s > %s)", fieldExpr, c.bind(node.Value)), nil
 	}
 	if num, ok := toFloat64(node.Value); ok {
-		return fmt.Sprintf("((%s)::double precision > %s)", metadataPathTextExpr(fieldExpr, path), c.bind(num)), nil
+		return c.compileMetadataNumericCompare(fieldExpr, path, ">", num), nil
 	}
 	return fmt.Sprintf("(%s > %s)", metadataPathTextExpr(fieldExpr, path), c.bind(fmt.Sprint(node.Value))), nil
 }
@@ -137,11 +139,23 @@ func (c *filterCompiler) compileLt(node LtFilter) (string, error) {
 		return fmt.Sprintf("(%s < %s)", fieldExpr, c.bind(node.Value)), nil
 	}
 	if num, ok := toFloat64(node.Value); ok {
-		return fmt.Sprintf("((%s)::double precision < %s)", metadataPathTextExpr(fieldExpr, path), c.bind(num)), nil
+		return c.compileMetadataNumericCompare(fieldExpr, path, "<", num), nil
 	}
 	return fmt.Sprintf("(%s < %s)", metadataPathTextExpr(fieldExpr, path), c.bind(fmt.Sprint(node.Value))), nil
 }
 
+func (c *filterCompiler) compileMetadataNumericCompare(metadataExpr string, path []string, op string, value float64) string {
+	textExpr := metadataPathTextExpr(metadataExpr, path)
+	return fmt.Sprintf(
+		"(CASE WHEN (%s) ~ %s THEN ((%s)::double precision %s %s) ELSE FALSE END)",
+		textExpr,
+		singleQuoted(numericTextPattern),
+		textExpr,
+		op,
+		c.bind(value),
+	)
+}
+
 func (c *filterCompiler) compileExists(node ExistsFilter) (string, error) {
 	fieldExpr, isMetadata, path, err := c.resolveField(node.Field)
 	if err != nil {

vectordata/filter_sql_test.go

@@ -34,7 +34,7 @@ func TestCompileFilterSQL_Complex(t *testing.T) {
 		t.Fatalf("CompileFilterSQL error: %v", err)
 	}
 
-	expectedSQL := `(("id" = $1) AND (((jsonb_extract_path_text("metadata", 'rank'))::double precision > $2) OR (("metadata" #> ARRAY['flags', 'pinned']) IS NOT NULL)))`
+	expectedSQL := `(("id" = $1) AND ((CASE WHEN (jsonb_extract_path_text("metadata", 'rank')) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)([eE][+-]?[0-9]+)?$' THEN ((jsonb_extract_path_text("metadata", 'rank'))::double precision > $2) ELSE FALSE END) OR (("metadata" #> ARRAY['flags', 'pinned']) IS NOT NULL)))`
 	if sql != expectedSQL {
 		t.Fatalf("unexpected SQL\nwant: %s\n got: %s", expectedSQL, sql)
 	}
@@ -49,6 +49,30 @@ func TestCompileFilterSQL_Complex(t *testing.T) {
 	}
 }
 
+func TestCompileFilterSQL_MetadataGtNumericUsesSafeCast(t *testing.T) {
+	// Arrange
+	filter := Gt(Metadata("rank"), 10)
+
+	// Act
+	sql, args, next, err := CompileFilterSQL(filter, testFilterConfig(), 1)
+
+	// Assert
+	if err != nil {
+		t.Fatalf("CompileFilterSQL error: %v", err)
+	}
+	expectedSQL := `(CASE WHEN (jsonb_extract_path_text("metadata", 'rank')) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)([eE][+-]?[0-9]+)?$' THEN ((jsonb_extract_path_text("metadata", 'rank'))::double precision > $1) ELSE FALSE END)`
+	if sql != expectedSQL {
+		t.Fatalf("unexpected SQL\nwant: %s\n got: %s", expectedSQL, sql)
+	}
+	expectedArgs := []any{float64(10)}
+	if !reflect.DeepEqual(args, expectedArgs) {
+		t.Fatalf("unexpected args\nwant: %#v\n got: %#v", expectedArgs, args)
+	}
+	if next != 2 {
+		t.Fatalf("unexpected next arg index: %d", next)
+	}
+}
+
 func TestCompileFilterSQL_StartArgOffset(t *testing.T) {
 	// Arrange
 	filter := Eq(Column("content"), "hello")