filer: skip COLLATE "C" list fallback on CockroachDB (#10015)

* filer: skip COLLATE "C" list fallback on CockroachDB

CockroachDB string comparison is already byte-ordered, so wrapping the
list queries in COLLATE "C" can never change result order. It reports
datcollate=en_US.utf8 regardless of how the database was created, so the
collation check always misfires and forces the fallback. On 22.1 and
older COLLATE "C" is rejected as an invalid locale, turning every filer
list query into a hard failure. Detect the backend via version() and
keep the default ordering.

* filer: test CockroachDB collation detection

Cover the CockroachDB skip path and the locale-aware Postgres force path
with go-sqlmock.

go.mod

@@ -114,6 +114,7 @@ require (
 require (
 	cloud.google.com/go/kms v1.31.0
 	github.com/Azure/azure-sdk-for-go/sdk/keyvault/azkeys v0.10.0
+	github.com/DATA-DOG/go-sqlmock v1.5.2
 	github.com/Jille/raft-grpc-transport v1.6.1
 	github.com/ThreeDotsLabs/watermill v1.5.1
 	github.com/a-h/templ v0.3.1020

go.sum

@@ -1468,6 +1468,7 @@ github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRt
 github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
+github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46/go.mod h1:yyMNCyc/Ib3bDTKd379tNMpB/7/H5TjM2Y9QJ5THLbE=
 github.com/klauspost/asmfmt v1.3.2 h1:4Ri7ox3EwapiOjCki+hw14RyKk201CN4rzyCJRFLpK4=
 github.com/klauspost/asmfmt v1.3.2/go.mod h1:AG8TuvYojzulgDAMCnYn50l/5QV3Bs/tp6j0HLHbNSE=
 github.com/klauspost/compress v1.15.9/go.mod h1:PhcZ0MbTNciWF3rruxRgKxI5NkcHHrHUDtV4Yw2GlzU=

weed/filer/postgres/postgres_collation.go

@@ -12,6 +12,11 @@ import (
 // filemeta.name collation is locale-aware, so S3 ListObjectsV2 stays
 // lexicographic. The fallback costs a sort, so warn the operator to fix it.
 func ConfigureListOrdering(db *sql.DB, gen *SqlGenPostgres) {
+	if isCockroachDB(db) {
+		// CockroachDB string comparison is already byte-ordered, so the
+		// fallback is redundant; older versions also reject COLLATE "C".
+		return
+	}
 	collation, isBinary, err := nameColumnCollation(db)
 	if err != nil {
 		glog.V(1).Infof("postgres: skip filemeta.name collation check: %v", err)
@@ -24,6 +29,17 @@ func ConfigureListOrdering(db *sql.DB, gen *SqlGenPostgres) {
 	glog.Warningf(`postgres: filemeta.name collation %q is not byte-ordered, so S3 list order is not byte-lexicographic and clients that merge sorted listings may report spurious diffs. Falling back to a slower COLLATE "C" sort; declare the name column COLLATE "C" (or use a C/C.UTF-8 database collation) for correct, indexed ordering.`, collation)
 }
 
+// isCockroachDB reports whether db is a CockroachDB backend, whose version()
+// string carries "CockroachDB". On any error we assume real Postgres.
+func isCockroachDB(db *sql.DB) bool {
+	var version string
+	if err := db.QueryRow("SELECT version()").Scan(&version); err != nil {
+		glog.V(1).Infof("postgres: skip CockroachDB detection: %v", err)
+		return false
+	}
+	return strings.Contains(version, "CockroachDB")
+}
+
 func nameColumnCollation(db *sql.DB) (collation string, isBinary bool, err error) {
 	// information_schema reports NULL for a column on the database default, so
 	// fall back to datcollate for the effective ordering.

weed/filer/postgres/postgres_collation_test.go

@@ -0,0 +1,49 @@
+package postgres
+
+import (
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+)
+
+func TestConfigureListOrderingSkipsCockroachDB(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+
+	mock.ExpectQuery("SELECT version\\(\\)").WillReturnRows(
+		sqlmock.NewRows([]string{"version"}).AddRow("CockroachDB CCL v22.1.22 (x86_64-pc-linux-gnu)"))
+
+	gen := &SqlGenPostgres{}
+	ConfigureListOrdering(db, gen)
+	if gen.ForceBinaryCollation {
+		t.Fatal("CockroachDB default ordering is byte-ordered; COLLATE \"C\" must not be forced")
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestConfigureListOrderingForcesBinaryOnLocalePostgres(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+
+	mock.ExpectQuery("SELECT version\\(\\)").WillReturnRows(
+		sqlmock.NewRows([]string{"version"}).AddRow("PostgreSQL 16.2 on x86_64-pc-linux-gnu"))
+	mock.ExpectQuery("datcollate").WillReturnRows(
+		sqlmock.NewRows([]string{"collation_name", "datcollate"}).AddRow(nil, "en_US.UTF-8"))
+
+	gen := &SqlGenPostgres{}
+	ConfigureListOrdering(db, gen)
+	if !gen.ForceBinaryCollation {
+		t.Fatal("locale-aware Postgres collation must force byte ordering")
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}