diff --git a/core/record_field_resolver.go b/core/record_field_resolver.go index 6e68960c..da0759dd 100644 --- a/core/record_field_resolver.go +++ b/core/record_field_resolver.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/pocketbase/dbx" + "github.com/pocketbase/pocketbase/tools/inflector" "github.com/pocketbase/pocketbase/tools/search" "github.com/pocketbase/pocketbase/tools/security" "github.com/pocketbase/pocketbase/tools/types" @@ -191,6 +192,7 @@ func (r *RecordFieldResolver) updateQueryWithDeduplicateConstraint(query *dbx.Se return } + // already has the group by registered var groupByCol = r.baseCollection.Name if r.baseCollectionAlias != "" { groupByCol = r.baseCollectionAlias @@ -203,16 +205,52 @@ func (r *RecordFieldResolver) updateQueryWithDeduplicateConstraint(query *dbx.Se // when deemed safe (GROUP BY could have different execution order compared to DISTINCT), // prefer GROUP BY to deduplicate only on the id field instead of all columns // so that the size of a single row wouldn't matter that much - if len(info.GroupBy) == 0 && - info.Having == nil && - (len(info.Selects) == 0 || - (len(info.Selects) == 1 && strings.HasSuffix(info.Selects[0], ".*"))) { + if preferGroupBy(info, groupByCol) { query.GroupBy(groupByCol) } else { query.Distinct(true) } } +func preferGroupBy(info *dbx.QueryInfo, fullUnquotedGroupByCol string) bool { + if len(info.GroupBy) != 0 { + return false + } + + if info.Having != nil { + return false + } + + // dbx fallbacks to * if not set + if len(info.Selects) == 0 { + return true + } + + if len(info.Selects) != 1 { + return false + } + + identifier := info.Selects[0] + + if identifier == "*" || identifier == fullUnquotedGroupByCol { + return true + } + + // try again as direct col match in an unquoted column format + identifier = inflector.Columnify(identifier) + if identifier == fullUnquotedGroupByCol { + return true + } + + // remains table.* to check + // (aliased columns for now are ignored as they could be represented by expressions) + if !strings.HasSuffix(identifier, ".*") { + return false + } + + return strings.HasPrefix(fullUnquotedGroupByCol, strings.TrimSuffix(identifier, "*")) +} + // Resolve implements `search.FieldResolver` interface. // // Example of some resolvable fieldName formats: diff --git a/core/record_field_resolver_test.go b/core/record_field_resolver_test.go index e7251a72..5416f556 100644 --- a/core/record_field_resolver_test.go +++ b/core/record_field_resolver_test.go @@ -934,3 +934,185 @@ func TestRecordFieldResolverResolveStaticRequestInfoFields(t *testing.T) { t.Fatalf("Expected the original authRecord email to not be exported, got %q", v) } } + +func TestRecordFieldResolverDeduplicateUpdateRules(t *testing.T) { + t.Parallel() + + app, _ := tests.NewTestApp() + defer app.Cleanup() + + collection, err := app.FindCollectionByNameOrId("demo4") + if err != nil { + t.Fatal(err) + } + + r := core.NewRecordFieldResolver(app, collection, nil, false) + + // register a dummy relation join + _, err = r.Resolve("self_rel_one.created") + if err != nil { + t.Fatalf("failed to register a dummy relation join: %v", err) + } + + t.Run("with existing DISTINCT", func(t *testing.T) { + query := app.DB().Select().Distinct(true) + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if !info.Distinct { + t.Fatal("Expected DISTINCT to be preserved, got false") + } + + if len(info.GroupBy) != 0 { + t.Fatalf("Expected no GROUP BY, got %v", info.GroupBy) + } + }) + + t.Run("without existing DISTINCT", func(t *testing.T) { + query := app.DB().Select() + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if info.Distinct { + t.Fatal("Unexpected DISTINCT") + } + + if len(info.GroupBy) != 1 || info.GroupBy[0] != "demo4.id" { + t.Fatalf("Missing expected GROUP BY %s, got %v", "demo4.id", info.GroupBy) + } + }) + + t.Run("with existing deduplicate GROUP BY", func(t *testing.T) { + query := app.DB().Select().GroupBy("demo4.id") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if info.Distinct { + t.Fatal("Unexpected DISTINCT") + } + + if len(info.GroupBy) != 1 || info.GroupBy[0] != "demo4.id" { + t.Fatalf("Missing expected GROUP BY %s, got %v", "demo4.id", info.GroupBy) + } + }) + + t.Run("with custom GROUP BY expression", func(t *testing.T) { + query := app.DB().Select().GroupBy("rowid") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if !info.Distinct { + t.Fatal("Missing expected DISTINCT") + } + + if len(info.GroupBy) != 1 || info.GroupBy[0] != "rowid" { + t.Fatalf("Missing expected GROUP BY %s, got %v", "rowid", info.GroupBy) + } + }) + + t.Run("with no prefixed wildcard SELECT column", func(t *testing.T) { + query := app.DB().Select("*") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if info.Distinct { + t.Fatal("Unexpected DISTINCT") + } + + if len(info.GroupBy) != 1 || info.GroupBy[0] != "demo4.id" { + t.Fatalf("Missing expected GROUP BY %s, got %v", "demo4.id", info.GroupBy) + } + }) + + t.Run("with matching prefixed wildcard SELECT column", func(t *testing.T) { + query := app.DB().Select("demo4.*") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if info.Distinct { + t.Fatal("Unexpected DISTINCT") + } + + if len(info.GroupBy) != 1 || info.GroupBy[0] != "demo4.id" { + t.Fatalf("Missing expected GROUP BY %s, got %v", "demo4.id", info.GroupBy) + } + }) + + t.Run("with custom table prefixed wildcard SELECT column", func(t *testing.T) { + query := app.DB().Select("abc.*") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if !info.Distinct { + t.Fatal("Missing expected DISTINCT") + } + + if len(info.GroupBy) != 0 { + t.Fatalf("Expected no GROUP BY, got %v", info.GroupBy) + } + }) + + t.Run("with exact SELECT column", func(t *testing.T) { + query := app.DB().Select("`demo4`.id") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if info.Distinct { + t.Fatal("Unexpected DISTINCT") + } + + if len(info.GroupBy) != 1 || info.GroupBy[0] != "demo4.id" { + t.Fatalf("Missing expected GROUP BY %s, got %v", "demo4.id", info.GroupBy) + } + }) + + t.Run("with > 1 SELECT columns", func(t *testing.T) { + query := app.DB().Select("`demo4`.id", "demo4.created") + + if err := r.UpdateQuery(query); err != nil { + t.Fatalf("UpdateQuery failed: %v", err) + } + + info := query.Info() + + if !info.Distinct { + t.Fatal("Missing expected DISTINCT") + } + + if len(info.GroupBy) != 0 { + t.Fatalf("Expected no GROUP BY, got %v", info.GroupBy) + } + }) +}