From 48cd71066ba0cba99cf9e7fdae6b1a9778efeec5 Mon Sep 17 00:00:00 2001 From: Chengxuan Xing Date: Wed, 7 Jan 2026 12:42:25 +0000 Subject: [PATCH] adding support for DISTINCT ON for postgres DB Signed-off-by: Chengxuan Xing --- pkg/dbsql/crud.go | 39 +++++- pkg/dbsql/crud_test.go | 164 ++++++++++++++++++++++- pkg/dbsql/database.go | 34 ++++- pkg/dbsql/database_test.go | 111 +++++++++++++++- pkg/dbsql/filter_sql.go | 49 ++++++- pkg/dbsql/filter_sql_test.go | 204 ++++++++++++++++++++++++++++- pkg/ffapi/filter.go | 27 +++- pkg/ffapi/filter_test.go | 118 ++++++++++++++++- pkg/i18n/en_base_error_messages.go | 3 +- 9 files changed, 737 insertions(+), 12 deletions(-) diff --git a/pkg/dbsql/crud.go b/pkg/dbsql/crud.go index f9a928d2..2e72332b 100644 --- a/pkg/dbsql/crud.go +++ b/pkg/dbsql/crud.go @@ -1,4 +1,4 @@ -// Copyright © 2024 Kaleido, Inc. +// Copyright © 2024 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -785,13 +785,48 @@ func (c *CrudBase[T]) getManyScoped(ctx context.Context, tableFrom string, fi *f if err != nil { return nil, nil, err } + + // Apply ReadQueryModifier first (before wrapping with DISTINCT ON) if c.ReadQueryModifier != nil { if query, err = c.ReadQueryModifier(query); err != nil { return nil, nil, err } } - rows, tx, err := c.DB.Query(ctx, c.Table, query) + // Apply placeholder format to the query + query = query.PlaceholderFormat(c.DB.features.PlaceholderFormat) + + // Apply DISTINCT ON wrapper for PostgreSQL if needed + var finalQuery sq.Sqlizer = query + if len(fi.DistinctOn) > 0 { + if c.DB.provider == nil || c.DB.provider.Name() != "postgres" { + providerName := "unknown" + if c.DB.provider != nil { + providerName = c.DB.provider.Name() + } + return nil, nil, i18n.NewError(ctx, i18n.MsgDistinctOnNotSupported, providerName) + } + distinctOnFields := make([]string, len(fi.DistinctOn)) + for i, do := range fi.DistinctOn { + // Map the field name using the FilterFieldMap + mappedField := do + if c.FilterFieldMap != nil { + if mf, ok := c.FilterFieldMap[do]; ok { + mappedField = mf + } + } + if c.ReadTableAlias != "" && !strings.Contains(mappedField, ".") { + mappedField = fmt.Sprintf("%s.%s", c.ReadTableAlias, mappedField) + } + distinctOnFields[i] = mappedField + } + finalQuery = &distinctOnSqlizer{ + inner: query, + distinctOn: distinctOnFields, + } + } + + rows, tx, err := c.DB.RunAsQueryTx(ctx, c.Table, nil, finalQuery) if err != nil { return nil, nil, err } diff --git a/pkg/dbsql/crud_test.go b/pkg/dbsql/crud_test.go index ba0be31e..26e263e8 100644 --- a/pkg/dbsql/crud_test.go +++ b/pkg/dbsql/crud_test.go @@ -1,4 +1,4 @@ -// Copyright © 2023 Kaleido, Inc. +// Copyright © 2023 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -112,6 +112,7 @@ var LinkableQueryFactory = &ffapi.QueryFields{ "id": &ffapi.UUIDField{}, "created": &ffapi.TimeField{}, "updated": &ffapi.TimeField{}, + "ns": &ffapi.StringField{}, "crud": &ffapi.UUIDField{}, } @@ -1525,3 +1526,164 @@ func TestCustomIDColumn(t *testing.T) { } tc.Validate() } + +// postgresMockProvider wraps MockProvider to return "postgres" as the name for DISTINCT ON testing +type postgresMockProvider struct { + *MockProvider +} + +func (p *postgresMockProvider) Name() string { + return "postgres" +} + +func newPostgresMockProvider() (*postgresMockProvider, sqlmock.Sqlmock) { + mp := NewMockProvider() + pgmp := &postgresMockProvider{MockProvider: mp} + _ = pgmp.Database.Init(context.Background(), pgmp, pgmp.config) + return pgmp, pgmp.mdb +} + +func TestGetManyWithDistinctOnPostgreSQL(t *testing.T) { + db, mock := newPostgresMockProvider() + tc := newCRUDCollection(&db.Database, "ns1") + + // Test DISTINCT ON with a single field + filter := CRUDableQueryFactory.NewFilter(context.Background()). + DistinctOn("ns"). + And() + + // Expect query with DISTINCT ON + mock.ExpectQuery(`SELECT DISTINCT ON \(.*ns.*\)`). + WillReturnRows(sqlmock.NewRows([]string{"seq", "id", "created", "updated", "ns", "name", "field1", "field2", "field3", "field4", "field5", "field6"}). + AddRow(1, fftypes.NewUUID().String(), fftypes.Now().String(), fftypes.Now().String(), "ns1", "test", "test", nil, nil, nil, nil, nil)) + + results, _, err := tc.GetMany(context.Background(), filter) + assert.NoError(t, err) + assert.NotNil(t, results) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetManyWithDistinctOnMultipleFieldsPostgreSQL(t *testing.T) { + db, mock := newPostgresMockProvider() + tc := newCRUDCollection(&db.Database, "ns1") + + // Test DISTINCT ON with multiple fields + filter := CRUDableQueryFactory.NewFilter(context.Background()). + DistinctOn("ns", "name"). + And() + + // Expect query with DISTINCT ON for multiple fields + mock.ExpectQuery(`SELECT DISTINCT ON \(.*ns.*name.*\)`). + WillReturnRows(sqlmock.NewRows([]string{"seq", "id", "created", "updated", "ns", "name", "field1", "field2", "field3", "field4", "field5", "field6"}). + AddRow(1, fftypes.NewUUID().String(), fftypes.Now().String(), fftypes.Now().String(), "ns1", "test", "test", nil, nil, nil, nil, nil)) + + results, _, err := tc.GetMany(context.Background(), filter) + assert.NoError(t, err) + assert.NotNil(t, results) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetManyWithDistinctOnAndReadTableAliasPostgreSQL(t *testing.T) { + db, mock := newPostgresMockProvider() + linkables := newLinkableCollection(&db.Database, "ns1") + + // Test DISTINCT ON with ReadTableAlias + filter := LinkableQueryFactory.NewFilter(context.Background()). + DistinctOn("ns"). + And() + + // Expect query with DISTINCT ON using table alias + // The regex should match: SELECT DISTINCT ON (l.ns) ... + mock.ExpectQuery(`SELECT.*DISTINCT ON.*l\.ns`). + WillReturnRows(sqlmock.NewRows([]string{"seq", "id", "created", "updated", "ns", "desc", "crud_id", "c.field1", "c.field2", "c.field3"}). + AddRow(1, fftypes.NewUUID().String(), fftypes.Now().String(), fftypes.Now().String(), "ns1", "desc", nil, "", nil, nil)) + + results, _, err := linkables.GetMany(context.Background(), filter) + assert.NoError(t, err) + assert.NotNil(t, results) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetManyWithDistinctOnAndReadQueryModifierPostgreSQL(t *testing.T) { + db, mock := newPostgresMockProvider() + tc := newCRUDCollection(&db.Database, "ns1") + + // Set up a ReadQueryModifier + tc.ReadQueryModifier = func(sb sq.SelectBuilder) (sq.SelectBuilder, error) { + return sb.Where(sq.Eq{"ns": "ns1"}), nil + } + + // Test DISTINCT ON with ReadQueryModifier + filter := CRUDableQueryFactory.NewFilter(context.Background()). + DistinctOn("ns"). + And() + + // Expect query with DISTINCT ON - modifier should be applied to inner query before wrapping + mock.ExpectQuery(`SELECT DISTINCT ON \(.*ns.*\)`). + WillReturnRows(sqlmock.NewRows([]string{"seq", "id", "created", "updated", "ns", "name", "field1", "field2", "field3", "field4", "field5", "field6"}). + AddRow(1, fftypes.NewUUID().String(), fftypes.Now().String(), fftypes.Now().String(), "ns1", "test", "test", nil, nil, nil, nil, nil)) + + results, _, err := tc.GetMany(context.Background(), filter) + assert.NoError(t, err) + assert.NotNil(t, results) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetManyWithDistinctOnNonPostgreSQL(t *testing.T) { + // Test that DISTINCT ON throws an error for non-PostgreSQL databases + db, mock := NewMockProvider().UTInit() + tc := newCRUDCollection(&db.Database, "ns1") + + // Test DISTINCT ON with non-postgres provider (should return error) + filter := CRUDableQueryFactory.NewFilter(context.Background()). + DistinctOn("ns"). + And() + + // Should return error, no query should be executed + _, _, err := tc.GetMany(context.Background(), filter) + assert.Error(t, err) + assert.Regexp(t, "FF00258", err) + assert.Contains(t, err.Error(), "DISTINCT ON is only supported for PostgreSQL") + assert.Contains(t, err.Error(), "mockdb") + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetManyWithDistinctOnAndFilterFieldMapPostgreSQL(t *testing.T) { + db, mock := newPostgresMockProvider() + tc := newCRUDCollection(&db.Database, "ns1") + + // Test DISTINCT ON with field mapping (f1 -> field1) + filter := CRUDableQueryFactory.NewFilter(context.Background()). + DistinctOn("f1"). + And() + + // Expect query with DISTINCT ON using mapped field name + mock.ExpectQuery(`SELECT DISTINCT ON \(.*field1.*\)`). + WillReturnRows(sqlmock.NewRows([]string{"seq", "id", "created", "updated", "ns", "name", "field1", "field2", "field3", "field4", "field5", "field6"}). + AddRow(1, fftypes.NewUUID().String(), fftypes.Now().String(), fftypes.Now().String(), "ns1", "test", "test", nil, nil, nil, nil, nil)) + + results, _, err := tc.GetMany(context.Background(), filter) + assert.NoError(t, err) + assert.NotNil(t, results) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetManyWithDistinctOnReadQueryModifierError(t *testing.T) { + db, mock := newPostgresMockProvider() + tc := newCRUDCollection(&db.Database, "ns1") + + // Set up a ReadQueryModifier that returns an error + tc.ReadQueryModifier = func(sb sq.SelectBuilder) (sq.SelectBuilder, error) { + return sb, fmt.Errorf("modifier error") + } + + // Test DISTINCT ON with ReadQueryModifier error + filter := CRUDableQueryFactory.NewFilter(context.Background()). + DistinctOn("ns"). + And() + + _, _, err := tc.GetMany(context.Background(), filter) + assert.Error(t, err) + assert.Contains(t, err.Error(), "modifier error") + assert.NoError(t, mock.ExpectationsWereMet()) +} diff --git a/pkg/dbsql/database.go b/pkg/dbsql/database.go index df0434d2..5658ad0b 100644 --- a/pkg/dbsql/database.go +++ b/pkg/dbsql/database.go @@ -1,4 +1,4 @@ -// Copyright © 2024 Kaleido, Inc. +// Copyright © 2024 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -20,6 +20,7 @@ import ( "context" "database/sql" "fmt" + "strings" "time" sq "github.com/Masterminds/squirrel" @@ -205,10 +206,39 @@ func (s *Database) QueryTx(ctx context.Context, table string, tx *TXWrapper, q s return s.RunAsQueryTx(ctx, table, tx, q.PlaceholderFormat(s.features.PlaceholderFormat)) } +// distinctOnSqlizer wraps a SelectBuilder to add PostgreSQL DISTINCT ON support +type distinctOnSqlizer struct { + inner sq.SelectBuilder + distinctOn []string +} + +func (d *distinctOnSqlizer) ToSql() (string, []interface{}, error) { + sql, args, err := d.inner.ToSql() + if err != nil { + return sql, args, err + } + // Replace SELECT with SELECT DISTINCT ON (fields) + if len(d.distinctOn) > 0 { + distinctOnClause := fmt.Sprintf("DISTINCT ON (%s) ", strings.Join(d.distinctOn, ", ")) + sql = strings.Replace(sql, "SELECT ", fmt.Sprintf("SELECT %s", distinctOnClause), 1) + } + return sql, args, nil +} + func (s *Database) RunAsQueryTx(ctx context.Context, table string, tx *TXWrapper, q sq.Sqlizer) (*sql.Rows, *TXWrapper, error) { l := log.L(ctx) - sqlQuery, args, err := q.ToSql() + + // Check if this is a distinctOnSqlizer wrapper + var sqlQuery string + var args []interface{} + var err error + if dos, ok := q.(*distinctOnSqlizer); ok { + sqlQuery, args, err = dos.ToSql() + } else { + sqlQuery, args, err = q.ToSql() + } + if err != nil { return nil, tx, i18n.WrapError(ctx, err, i18n.MsgDBQueryBuildFailed) } diff --git a/pkg/dbsql/database_test.go b/pkg/dbsql/database_test.go index 49ac5fef..66c26031 100644 --- a/pkg/dbsql/database_test.go +++ b/pkg/dbsql/database_test.go @@ -1,4 +1,4 @@ -// Copyright © 2023 Kaleido, Inc. +// Copyright © 2023 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -622,3 +622,112 @@ func TestExecTxFail(t *testing.T) { _, err = s.ExecTx(ctx, "mytable", tx, sqlQuery, args) assert.Regexp(t, "pop", err) } + +func TestDistinctOnSqlizerToSql(t *testing.T) { + // Test that distinctOnSqlizer correctly transforms SQL + s, _ := NewMockProvider().UTInit() + innerQuery := sq.Select("id", "name", "tag").From("mytable").Where(sq.Eq{"tag": "test"}).PlaceholderFormat(s.features.PlaceholderFormat) + dos := &distinctOnSqlizer{ + inner: innerQuery, + distinctOn: []string{"tag", "name"}, + } + + sql, args, err := dos.ToSql() + assert.NoError(t, err) + // Should contain DISTINCT ON clause + assert.Contains(t, sql, "SELECT DISTINCT ON (tag, name)") + assert.Contains(t, sql, "FROM mytable") + assert.Contains(t, sql, "WHERE") + assert.Contains(t, sql, "tag = $1") + assert.Len(t, args, 1) + assert.Equal(t, "test", args[0]) +} + +func TestDistinctOnSqlizerToSqlEmptyDistinctOn(t *testing.T) { + // Test that distinctOnSqlizer with empty distinctOn doesn't add DISTINCT ON + innerQuery := sq.Select("id", "name").From("mytable") + dos := &distinctOnSqlizer{ + inner: innerQuery, + distinctOn: []string{}, + } + + sql, args, err := dos.ToSql() + assert.NoError(t, err) + // Should NOT contain DISTINCT ON clause + assert.NotContains(t, sql, "DISTINCT ON") + assert.Contains(t, sql, "SELECT") + assert.Contains(t, sql, "FROM mytable") + assert.Empty(t, args) +} + +func TestDistinctOnSqlizerToSqlInnerError(t *testing.T) { + // Test that distinctOnSqlizer propagates errors from inner query + // Use an empty SelectBuilder which will fail when ToSql is called + innerQuery := sq.SelectBuilder{} + dos := &distinctOnSqlizer{ + inner: innerQuery, + distinctOn: []string{"id"}, + } + + _, _, err := dos.ToSql() + // The error should be propagated from the inner query + assert.Error(t, err) +} + +func TestRunAsQueryTxWithDistinctOnSqlizer(t *testing.T) { + // Test that RunAsQueryTx correctly handles distinctOnSqlizer + s, mdb := NewMockProvider().UTInit() + innerQuery := sq.Select("id", "name").From("mytable") + dos := &distinctOnSqlizer{ + inner: innerQuery, + distinctOn: []string{"name"}, + } + + mdb.ExpectQuery("SELECT DISTINCT ON \\(name\\) id, name FROM mytable"). + WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow("1", "test")) + + rows, tx, err := s.RunAsQueryTx(context.Background(), "mytable", nil, dos) + assert.NoError(t, err) + assert.NotNil(t, rows) + assert.Nil(t, tx) // No transaction passed, so tx should be nil + rows.Close() + assert.NoError(t, mdb.ExpectationsWereMet()) +} + +func TestRunAsQueryTxWithDistinctOnSqlizerInTransaction(t *testing.T) { + // Test that RunAsQueryTx correctly handles distinctOnSqlizer within a transaction + s, mdb := NewMockProvider().UTInit() + mdb.ExpectBegin() + ctx, tx, _, err := s.BeginOrUseTx(context.Background()) + assert.NoError(t, err) + + innerQuery := sq.Select("id", "name").From("mytable") + dos := &distinctOnSqlizer{ + inner: innerQuery, + distinctOn: []string{"name"}, + } + + mdb.ExpectQuery("SELECT DISTINCT ON \\(name\\) id, name FROM mytable"). + WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow("1", "test")) + + rows, returnedTx, err := s.RunAsQueryTx(ctx, "mytable", tx, dos) + assert.NoError(t, err) + assert.NotNil(t, rows) + assert.Equal(t, tx, returnedTx) // Transaction should be returned + rows.Close() + assert.NoError(t, mdb.ExpectationsWereMet()) +} + +func TestRunAsQueryTxWithDistinctOnSqlizerBuildError(t *testing.T) { + // Test that RunAsQueryTx handles SQL build errors from distinctOnSqlizer + s, _ := NewMockProvider().UTInit() + innerQuery := sq.SelectBuilder{} + dos := &distinctOnSqlizer{ + inner: innerQuery, + distinctOn: []string{"id"}, + } + + _, _, err := s.RunAsQueryTx(context.Background(), "mytable", nil, dos) + assert.Error(t, err) + assert.Regexp(t, "FF00174", err) // DBQueryBuildFailed +} diff --git a/pkg/dbsql/filter_sql.go b/pkg/dbsql/filter_sql.go index 14359ad2..0ac11a69 100644 --- a/pkg/dbsql/filter_sql.go +++ b/pkg/dbsql/filter_sql.go @@ -1,4 +1,4 @@ -// Copyright © 2025 Kaleido, Inc. +// Copyright © 2025 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -99,7 +99,8 @@ func (s *Database) FilterSelect(ctx context.Context, tableName string, sel sq.Se } func (s *Database) filterSelectFinalized(ctx context.Context, tableName string, sel sq.SelectBuilder, fi *ffapi.FilterInfo, typeMap map[string]string, defaultSort []interface{}, preconditions ...sq.Sqlizer) (sq.SelectBuilder, sq.Sqlizer, *ffapi.FilterInfo, error) { - if len(fi.Sort) == 0 { + sortNotProvided := len(fi.Sort) == 0 + if sortNotProvided { for _, s := range defaultSort { switch v := s.(type) { case string: @@ -118,6 +119,47 @@ func (s *Database) filterSelectFinalized(ctx context.Context, tableName string, sel = sel.Where(fop) + // Handle PostgreSQL DISTINCT ON - ensure ORDER BY starts with DISTINCT ON fields + if len(fi.DistinctOn) > 0 { + if s.provider == nil || s.provider.Name() != "postgres" { + providerName := "unknown" + if s.provider != nil { + providerName = s.provider.Name() + } + return sel, nil, nil, i18n.NewError(ctx, i18n.MsgDistinctOnNotSupported, providerName) + } + // Ensure ORDER BY starts with DISTINCT ON fields (PostgreSQL requirement) + distinctOnInSort := make(map[string]bool) + for _, doField := range fi.DistinctOn { + distinctOnInSort[doField] = false + } + + // Check which DISTINCT ON fields are already in sort + for _, sf := range fi.Sort { + if _, ok := distinctOnInSort[sf.Field]; ok { + distinctOnInSort[sf.Field] = true + } + } + + // Prepend missing DISTINCT ON fields to sort (they must come first) + newSort := make([]*ffapi.SortField, 0, len(fi.DistinctOn)+len(fi.Sort)) + for _, doField := range fi.DistinctOn { + if !distinctOnInSort[doField] { + newSort = append(newSort, &ffapi.SortField{Field: doField, Descending: false}) + } + } + // Add existing sort fields, ensuring DISTINCT ON fields come first + for _, sf := range fi.Sort { + if distinctOnInSort[sf.Field] { + // This is a DISTINCT ON field - prepend it to preserve sort direction + newSort = append([]*ffapi.SortField{sf}, newSort...) + } else { + newSort = append(newSort, sf) + } + } + fi.Sort = newSort + } + if len(fi.GroupBy) > 0 { groupByWithResolvedFieldName := make([]string, len(fi.GroupBy)) for i, gb := range fi.GroupBy { @@ -152,6 +194,9 @@ func (s *Database) filterSelectFinalized(ctx context.Context, tableName string, if fi.Limit > 0 { sel = sel.Limit(fi.Limit) } + + // Note: DISTINCT ON will be applied via distinctOnSqlizer wrapper in QueryTx + // The FilterInfo already has the DistinctOn fields stored for reference return sel, fop, fi, err } diff --git a/pkg/dbsql/filter_sql_test.go b/pkg/dbsql/filter_sql_test.go index 99932a3f..8732f28e 100644 --- a/pkg/dbsql/filter_sql_test.go +++ b/pkg/dbsql/filter_sql_test.go @@ -1,4 +1,4 @@ -// Copyright © 2021 Kaleido, Inc. +// Copyright © 2021 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -410,3 +410,205 @@ func TestFilterUpdateErr(t *testing.T) { _, err := s.FilterUpdate(context.Background(), q, f, nil) assert.Regexp(t, "FF00142", err) } + +func TestFilterSelectDistinctOnPostgreSQL(t *testing.T) { + // Test DISTINCT ON with PostgreSQL provider - should work + pgmp := &postgresMockProvider{MockProvider: NewMockProvider()} + _ = pgmp.Database.Init(context.Background(), pgmp, pgmp.config) + + fb := TestQueryFactory.NewFilter(context.Background()) + f := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("tag", "type") + + sel := squirrel.Select("*").From("mytable") + sel, _, _, err := pgmp.FilterSelect(context.Background(), "", sel, f, nil, []interface{}{"sequence"}) + assert.NoError(t, err) + + sqlFilter, args, err := sel.ToSql() + assert.NoError(t, err) + // DISTINCT ON fields should be prepended to ORDER BY + assert.Contains(t, sqlFilter, "ORDER BY tag, type, seq DESC") + assert.Equal(t, "tag1", args[0]) +} + +func TestFilterSelectDistinctOnNonPostgreSQL(t *testing.T) { + // Test DISTINCT ON with non-PostgreSQL provider - should error + s, _ := NewMockProvider().UTInit() + + fb := TestQueryFactory.NewFilter(context.Background()) + f := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("tag") + + sel := squirrel.Select("*").From("mytable") + _, _, _, err := s.FilterSelect(context.Background(), "", sel, f, nil, []interface{}{"sequence"}) + assert.Error(t, err) + assert.Regexp(t, "FF00258", err) + assert.Contains(t, err.Error(), "DISTINCT ON is only supported for PostgreSQL") + assert.Contains(t, err.Error(), "mockdb") +} + +func TestFilterSelectDistinctOnWithSortPostgreSQL(t *testing.T) { + // Test DISTINCT ON with existing sort - DISTINCT ON fields should come first + pgmp := &postgresMockProvider{MockProvider: NewMockProvider()} + _ = pgmp.Database.Init(context.Background(), pgmp, pgmp.config) + + fb := TestQueryFactory.NewFilter(context.Background()) + // Use a DISTINCT ON field that's NOT in the sort to avoid duplication + f := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("type").Sort("-tag") + + sel := squirrel.Select("*").From("mytable") + sel, _, _, err := pgmp.FilterSelect(context.Background(), "", sel, f, nil, []interface{}{"sequence"}) + assert.NoError(t, err) + + sqlFilter, args, err := sel.ToSql() + assert.NoError(t, err) + + orderByStart := -1 + for i := 0; i <= len(sqlFilter)-len("ORDER BY"); i++ { + if sqlFilter[i:i+len("ORDER BY")] == "ORDER BY" { + orderByStart = i + len("ORDER BY") + break + } + } + assert.Greater(t, orderByStart, 0, "ORDER BY clause should be present") + + if orderByStart > 0 { + orderByClause := sqlFilter[orderByStart:] + // Remove any trailing clauses (LIMIT, etc.) + if limitIdx := findSubstring(orderByClause, " LIMIT"); limitIdx > 0 { + orderByClause = orderByClause[:limitIdx] + } + // Verify required fields are present + assert.Contains(t, orderByClause, "type") + assert.Contains(t, orderByClause, "tag DESC") + // Verify type comes before tag + typePos := findSubstring(orderByClause, "type") + tagPos := findSubstring(orderByClause, "tag") + assert.Greater(t, tagPos, typePos, "type should come before tag in ORDER BY") + } + assert.Equal(t, "tag1", args[0]) +} + +func TestFilterSelectDistinctOnWithDistinctSortPostgreSQL(t *testing.T) { + // Test DISTINCT ON with existing sort - DISTINCT ON fields should come first + pgmp := &postgresMockProvider{MockProvider: NewMockProvider()} + _ = pgmp.Database.Init(context.Background(), pgmp, pgmp.config) + + fb := TestQueryFactory.NewFilter(context.Background()) + // Use a DISTINCT ON field that's NOT in the sort to avoid duplication + f := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("tag").Sort("type").Sort("-tag") + + sel := squirrel.Select("*").From("mytable") + sel, _, _, err := pgmp.FilterSelect(context.Background(), "", sel, f, nil, []interface{}{"sequence"}) + assert.NoError(t, err) + + sqlFilter, args, err := sel.ToSql() + assert.NoError(t, err) + + orderByStart := -1 + for i := 0; i <= len(sqlFilter)-len("ORDER BY"); i++ { + if sqlFilter[i:i+len("ORDER BY")] == "ORDER BY" { + orderByStart = i + len("ORDER BY") + break + } + } + assert.Greater(t, orderByStart, 0, "ORDER BY clause should be present") + + if orderByStart > 0 { + orderByClause := sqlFilter[orderByStart:] + // Remove any trailing clauses (LIMIT, etc.) + if limitIdx := findSubstring(orderByClause, " LIMIT"); limitIdx > 0 { + orderByClause = orderByClause[:limitIdx] + } + // Verify required fields are present + assert.Contains(t, orderByClause, "type") + assert.Contains(t, orderByClause, "tag DESC") + // Verify type comes before tag + typePos := findSubstring(orderByClause, "type") + tagPos := findSubstring(orderByClause, "tag DESC") + // tag should come before type + assert.Greater(t, typePos, tagPos, "tag should come before type in ORDER BY") + } + assert.Equal(t, "tag1", args[0]) +} + +func TestFilterSelectDistinctOnWithDefaultSortPostgreSQL(t *testing.T) { + // Test DISTINCT ON with existing sort - DISTINCT ON fields should come first + pgmp := &postgresMockProvider{MockProvider: NewMockProvider()} + _ = pgmp.Database.Init(context.Background(), pgmp, pgmp.config) + + fb := TestQueryFactory.NewFilter(context.Background()) + // Use a DISTINCT ON field that's NOT in the sort to avoid duplication + f := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("type") + + sel := squirrel.Select("*").From("mytable") + sel, _, _, err := pgmp.FilterSelect(context.Background(), "", sel, f, nil, []interface{}{"sequence"}) + assert.NoError(t, err) + + sqlFilter, args, err := sel.ToSql() + assert.NoError(t, err) + + orderByStart := -1 + for i := 0; i <= len(sqlFilter)-len("ORDER BY"); i++ { + if sqlFilter[i:i+len("ORDER BY")] == "ORDER BY" { + orderByStart = i + len("ORDER BY") + break + } + } + assert.Greater(t, orderByStart, 0, "ORDER BY clause should be present") + + if orderByStart > 0 { + orderByClause := sqlFilter[orderByStart:] + // Remove any trailing clauses (LIMIT, etc.) + if limitIdx := findSubstring(orderByClause, " LIMIT"); limitIdx > 0 { + orderByClause = orderByClause[:limitIdx] + } + // Verify required fields are present + assert.Contains(t, orderByClause, "type") + assert.Contains(t, orderByClause, "seq DESC") + // Verify type comes before tag + typePos := findSubstring(orderByClause, "type") + seqPos := findSubstring(orderByClause, "seq") + assert.Greater(t, seqPos, typePos, "seq should come after type in ORDER BY") + } + assert.Equal(t, "tag1", args[0]) +} + +// Helper function to find substring position +func findSubstring(s, substr string) int { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return i + } + } + return -1 +} + +func TestFilterSelectDistinctOnWithMultipleFieldsPostgreSQL(t *testing.T) { + // Test DISTINCT ON with multiple fields + pgmp := &postgresMockProvider{MockProvider: NewMockProvider()} + _ = pgmp.Database.Init(context.Background(), pgmp, pgmp.config) + + fb := TestQueryFactory.NewFilter(context.Background()) + f := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("type", "tag") + + sel := squirrel.Select("*").From("mytable") + sel, _, _, err := pgmp.FilterSelect(context.Background(), "", sel, f, nil, []interface{}{"sequence"}) + assert.NoError(t, err) + + sqlFilter, args, err := sel.ToSql() + assert.NoError(t, err) + // DISTINCT ON fields should be prepended to ORDER BY + assert.Contains(t, sqlFilter, "ORDER BY type, tag, seq DESC") + assert.Equal(t, "tag1", args[0]) +} diff --git a/pkg/ffapi/filter.go b/pkg/ffapi/filter.go index 16ab200e..2e9f43bf 100644 --- a/pkg/ffapi/filter.go +++ b/pkg/ffapi/filter.go @@ -1,4 +1,4 @@ -// Copyright © 2024 Kaleido, Inc. +// Copyright © 2024 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -34,6 +34,11 @@ type FilterModifiers[T any] interface { // GroupBy adds a set of fields to group rows that have the same values into summary rows. Not assured every persistence implementation will support this (doc DBs cannot) GroupBy(...string) T + // DistinctOn adds PostgreSQL-specific DISTINCT ON clause. Only supported for PostgreSQL databases. + // DISTINCT ON (field1, field2, ...) returns one row per unique combination of the specified fields. + // The ORDER BY clause must start with the DISTINCT ON fields. + DistinctOn(...string) T + // Ascending sort order Ascending() T @@ -255,6 +260,7 @@ type SortField struct { // into the underlying database mechanism's filter language type FilterInfo struct { GroupBy []string + DistinctOn []string // PostgreSQL-specific: DISTINCT ON (field1, field2, ...) RequiredFields []string Sort []*SortField Skip uint64 @@ -324,6 +330,9 @@ func (f *FilterInfo) String() string { if len(f.GroupBy) > 0 { val.WriteString(fmt.Sprintf(" groupBy=%s", strings.Join(f.GroupBy, ","))) } + if len(f.DistinctOn) > 0 { + val.WriteString(fmt.Sprintf(" distinctOn=%s", strings.Join(f.DistinctOn, ","))) + } if len(f.RequiredFields) > 0 { val.WriteString(fmt.Sprintf(" requiredFields=%s", strings.Join(f.RequiredFields, ","))) } @@ -365,6 +374,7 @@ type filterBuilder struct { queryFields QueryFields sort []*SortField groupBy []string + distinctOn []string requiredFields []string skip uint64 limit uint64 @@ -511,6 +521,7 @@ func (f *baseFilter) Finalize() (fi *FilterInfo, err error) { Value: value, Sort: f.fb.sort, GroupBy: f.fb.groupBy, + DistinctOn: f.fb.distinctOn, RequiredFields: f.fb.requiredFields, Skip: f.fb.skip, Limit: f.fb.limit, @@ -554,6 +565,20 @@ func (f *baseFilter) GroupBy(fields ...string) Filter { return f } +func (fb *filterBuilder) DistinctOn(fields ...string) FilterBuilder { + for _, field := range fields { + if _, ok := fb.queryFields[field]; ok { + fb.distinctOn = append(fb.distinctOn, field) + } + } + return fb +} + +func (f *baseFilter) DistinctOn(fields ...string) Filter { + _ = f.fb.DistinctOn(fields...) + return f +} + func (fb *filterBuilder) RequiredFields(fields ...string) FilterBuilder { for _, field := range fields { if _, ok := fb.queryFields[field]; ok { diff --git a/pkg/ffapi/filter_test.go b/pkg/ffapi/filter_test.go index cbd52d4b..caadb8e6 100644 --- a/pkg/ffapi/filter_test.go +++ b/pkg/ffapi/filter_test.go @@ -1,4 +1,4 @@ -// Copyright © 2021 Kaleido, Inc. +// Copyright © 2021 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -373,3 +373,119 @@ func TestValueFilterAccess(t *testing.T) { assert.Equal(t, "seq", fb.ValueFilter().Field()) assert.Equal(t, 12345, fb.ValueFilter().Value()) } + +func TestFilterDistinctOnSingleField(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("tag").Finalize() + assert.NoError(t, err) + assert.Contains(t, f.String(), "distinctOn=tag") + assert.Equal(t, []string{"tag"}, f.DistinctOn) +} + +func TestFilterDistinctOnMultipleFields(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("tag", "type").Finalize() + assert.NoError(t, err) + assert.Contains(t, f.String(), "distinctOn=tag,type") + assert.Equal(t, []string{"tag", "type"}, f.DistinctOn) +} + +func TestFilterDistinctOnWithSort(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("type").Sort("tag").Sort("-sequence").Finalize() + assert.NoError(t, err) + assert.Contains(t, f.String(), "distinctOn=type") + assert.Contains(t, f.String(), "sort=tag,-sequence") + assert.Equal(t, []string{"type"}, f.DistinctOn) +} + +func TestFilterDistinctOnWithGroupBy(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("type").GroupBy("type").Finalize() + assert.NoError(t, err) + assert.Contains(t, f.String(), "distinctOn=type") + assert.Contains(t, f.String(), "groupBy=type") + assert.Equal(t, []string{"type"}, f.DistinctOn) +} + +func TestFilterDistinctOnWithAllOptions(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("type", "tag"). + Sort("tag"). + GroupBy("type"). + Skip(10). + Limit(25). + RequiredFields("type", "tag"). + Finalize() + assert.NoError(t, err) + assert.Contains(t, f.String(), "distinctOn=type,tag") + assert.Contains(t, f.String(), "sort=tag") + assert.Contains(t, f.String(), "groupBy=type") + assert.Contains(t, f.String(), "skip=10") + assert.Contains(t, f.String(), "limit=25") + assert.Contains(t, f.String(), "requiredFields=type,tag") + assert.Equal(t, []string{"type", "tag"}, f.DistinctOn) +} + +func TestFilterDistinctOnInvalidField(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + // DISTINCT ON with invalid field should be ignored (not added to distinctOn) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn("invalidField", "tag").Finalize() + assert.NoError(t, err) + // Only valid field should be in distinctOn + assert.Equal(t, []string{"tag"}, f.DistinctOn) + assert.Contains(t, f.String(), "distinctOn=tag") +} + +func TestFilterDistinctOnOnFilterBuilder(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + // Test DistinctOn on FilterBuilder (before Finalize) + fb2 := fb.DistinctOn("tag", "type") + assert.NotNil(t, fb2) + + f, err := fb2.And( + fb.Eq("tag", "tag1"), + ).Finalize() + assert.NoError(t, err) + assert.Equal(t, []string{"tag", "type"}, f.DistinctOn) +} + +func TestFilterDistinctOnOnFilter(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + // Test DistinctOn on Filter (after calling And which returns Filter) + f := fb.And( + fb.Eq("tag", "tag1"), + ) + + // Call DistinctOn on the Filter interface + f2 := f.DistinctOn("type") + assert.NotNil(t, f2) + + // Finalize to get the FilterInfo + fi, err := f2.Finalize() + assert.NoError(t, err) + assert.Equal(t, []string{"type"}, fi.DistinctOn) +} + +func TestFilterDistinctOnEmptyFields(t *testing.T) { + fb := TestQueryFactory.NewFilter(context.Background()) + f, err := fb.And( + fb.Eq("tag", "tag1"), + ).DistinctOn().Finalize() + assert.NoError(t, err) + // Empty DistinctOn should result in empty slice + assert.Empty(t, f.DistinctOn) + assert.NotContains(t, f.String(), "distinctOn=") +} diff --git a/pkg/i18n/en_base_error_messages.go b/pkg/i18n/en_base_error_messages.go index 446a199c..9ae8bfaf 100644 --- a/pkg/i18n/en_base_error_messages.go +++ b/pkg/i18n/en_base_error_messages.go @@ -1,4 +1,4 @@ -// Copyright © 2024 Kaleido, Inc. +// Copyright © 2024 - 2026 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -192,4 +192,5 @@ var ( MsgRoutePathNotStartWithSlash = ffe("FF00255", "Route path '%s' must not start with '/'") MsgMethodNotAllowed = ffe("FF00256", "Method not allowed", http.StatusMethodNotAllowed) MsgInvalidLogLevel = ffe("FF00257", "Invalid log level: '%s'", http.StatusBadRequest) + MsgDistinctOnNotSupported = ffe("FF00258", "DISTINCT ON is only supported for PostgreSQL databases, but current provider is '%s'", 400) )