fix(catalog): include short values from JSON arrays in filter_options (kubeflow#1783)

Long JSON strings were excluded from filter_options output even if the
individual values in the array were short.

Signed-off-by: Paul Boyd <paul@pboyd.io>

Author: pboyd

catalog/internal/catalog/db_catalog.go

@@ -205,40 +205,25 @@ func (d *dbCatalogImpl) GetFilterOptions(ctx context.Context) (*apimodels.Filter
 			continue
 		}
 
-		// Deduplicate values
-		uniqueValues := make(map[string]bool)
-
-		// Parse JSON arrays for fields like language and tasks
-		for _, value := range values {
-			var arrayValues []string
-			if err := json.Unmarshal([]byte(value), &arrayValues); err == nil {
-				// Successfully parsed as array, add individual values
-				for _, v := range arrayValues {
-					uniqueValues[v] = true
-				}
-			} else {
-				// Not a JSON array
-				uniqueValues[value] = true
-			}
+		if len(values) == 0 {
+			continue
 		}
 
-		if len(uniqueValues) > 0 {
-			sortedValues := make([]string, 0, len(uniqueValues))
-			for v := range uniqueValues {
-				sortedValues = append(sortedValues, v)
-			}
-			sort.Strings(sortedValues)
+		sortedValues := make([]string, 0, len(values))
+		for _, v := range values {
+			sortedValues = append(sortedValues, v)
+		}
+		sort.Strings(sortedValues)
 
-			// Convert to []interface{} (supports future non-string filter types)
-			expandedValues := make([]interface{}, len(sortedValues))
-			for i, v := range sortedValues {
-				expandedValues[i] = v
-			}
+		// Convert to []any (supports future non-string filter types)
+		expandedValues := make([]any, len(sortedValues))
+		for i, v := range sortedValues {
+			expandedValues[i] = v
+		}
 
-			options[fieldName] = apimodels.FilterOption{
-				Type:   "string",
-				Values: expandedValues,
-			}
+		options[fieldName] = apimodels.FilterOption{
+			Type:   "string",
+			Values: expandedValues,
 		}
 	}
 

catalog/internal/db/service/catalog_model.go

@@ -12,6 +12,7 @@ import (
 	"github.com/kubeflow/model-registry/internal/db/schema"
 	"github.com/kubeflow/model-registry/internal/db/service"
 	"github.com/kubeflow/model-registry/internal/db/utils"
+	"github.com/lib/pq"
 	"gorm.io/gorm"
 )
 
@@ -237,49 +238,63 @@ func mapDataLayerToCatalogModel(modelCtx schema.Context, propertiesCtx []schema.
 func (r *CatalogModelRepositoryImpl) GetFilterableProperties(maxLength int) (map[string][]string, error) {
 	config := r.GetConfig()
 
+	if config.DB.Name() != "postgres" {
+		return nil, fmt.Errorf("GetFilterableProperties is only supported on PostgreSQL")
+	}
+
+	db, err := config.DB.DB()
+	if err != nil {
+		return nil, err
+	}
+
 	// Get table names using GORM utilities for database compatibility
 	contextTable := utils.GetTableName(config.DB, &schema.Context{})
 	propertyTable := utils.GetTableName(config.DB, &schema.ContextProperty{})
 
-	// Simplified query: get distinct property name/value pairs
 	query := fmt.Sprintf(`
-		SELECT DISTINCT cp.name, cp.string_value
-		FROM %s cp
-		WHERE cp.context_id IN (
-			SELECT id FROM %s WHERE type_id = ?
-		)
-		AND cp.name IN (
-			SELECT name FROM (
-				SELECT name, MAX(CHAR_LENGTH(string_value)) as max_len
-				FROM %s
-				WHERE context_id IN (
-					SELECT id FROM %s WHERE type_id = ?
+		SELECT name, array_agg(string_value) FROM (
+			SELECT
+				name,
+				string_value
+			FROM %s WHERE
+				context_id IN (
+					SELECT id FROM %s WHERE type_id=$1
 				)
 				AND string_value IS NOT NULL
 				AND string_value != ''
-				GROUP BY name
-			) AS field_lengths
-			WHERE max_len <= ?
+				AND string_value IS NOT JSON ARRAY
+
+			UNION
+
+			SELECT
+				name,
+				json_array_elements_text(string_value::json) AS string_value
+			FROM %s WHERE
+				context_id IN (
+					SELECT id FROM %s WHERE type_id=$1
+				)
+				AND string_value IS JSON ARRAY
 		)
-		AND cp.string_value IS NOT NULL
-		AND cp.string_value != ''
-		ORDER BY cp.name, cp.string_value
+		GROUP BY name HAVING MAX(CHAR_LENGTH(string_value)) <= $2
 	`, propertyTable, contextTable, propertyTable, contextTable)
 
-	type propertyRow struct {
-		Name        string
-		StringValue string
-	}
-
-	var rows []propertyRow
-	if err := config.DB.Raw(query, config.TypeID, config.TypeID, maxLength).Scan(&rows).Error; err != nil {
+	rows, err := db.Query(query, config.TypeID, maxLength)
+	if err != nil {
 		return nil, fmt.Errorf("error querying filterable properties: %w", err)
 	}
+	defer rows.Close()
+
+	result := map[string][]string{}
+	for rows.Next() {
+		var name string
+		var values pq.StringArray
+
+		err = rows.Scan(&name, &values)
+		if err != nil {
+			return nil, fmt.Errorf("error scanning filterable property row: %w", err)
+		}
 
-	// Aggregate values by property name in Go
-	result := make(map[string][]string)
-	for _, row := range rows {
-		result[row.Name] = append(result[row.Name], row.StringValue)
+		result[name] = []string(values)
 	}
 
 	return result, nil

catalog/internal/db/service/catalog_model_test.go

@@ -199,9 +199,9 @@ func TestCatalogModelRepository(t *testing.T) {
 		updateModel := &models.CatalogModelImpl{
 			ID: saved.GetID(), // Specify the ID for update
 			Attributes: &models.CatalogModelAttributes{
-				Name:                     apiutils.Of("updated-test-model"),
-				ExternalID:               apiutils.Of("updated-ext-456"),
-				CreateTimeSinceEpoch:     saved.GetAttributes().CreateTimeSinceEpoch, // Preserve create time
+				Name:                 apiutils.Of("updated-test-model"),
+				ExternalID:           apiutils.Of("updated-ext-456"),
+				CreateTimeSinceEpoch: saved.GetAttributes().CreateTimeSinceEpoch, // Preserve create time
 			},
 			Properties: &[]dbmodels.Properties{
 				{
@@ -413,7 +413,6 @@ func TestCatalogModelRepository(t *testing.T) {
 		assert.Contains(t, result, "license")
 		// Should exclude longer properties
 		assert.NotContains(t, result, "provider") // "HuggingFace" is > 10 chars
-		assert.NotContains(t, result, "language")
 		assert.NotContains(t, result, "tasks")
 	})
 }