added more UpdateQuery tests

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:

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)
+		}
+	})
+}