fix(backend): select the storage dialect by driver instead of always MySQL

[hsinhoyeh]

Part of #13956. This is a bug I found while scoping items 1 and 2 that the issue does not mention.

initDBClient ends with:

return storage.NewDB(newdb, storage.NewMySQLDialect())

The dialect is hardcoded, so every install gets MySQLDialect regardless of driverName — including PostgreSQL installs, which reach this line through the pgx branch a few lines above.

What that breaks

storage.SQLDialect exists specifically to paper over engine differences, and only MySQLDialect and SQLiteDialect implement it — there is no PostgreSQL implementation at all. So on a PostgreSQL install the stores emit MySQL-only SQL:

Method	Emitted (MySQL)	PostgreSQL needs
GroupConcat	GROUP_CONCAT(x SEPARATOR "y")	STRING_AGG(x::text, 'y')
Concat	CONCAT(a,"y",b)	CONCAT(a,'y',b) — double quotes are an identifier in PG
Upsert	ON DUPLICATE KEY UPDATE c=VALUES(c)	ON CONFLICT(k) DO UPDATE SET c=EXCLUDED.c
UpdateWithJointOrFrom	UPDATE t INNER JOIN u ...	UPDATE t SET ... FROM u ...
IsDuplicateError	asserts *mysql.MySQLError	*pgconn.PgError, SQLSTATE 23505

IsDuplicateError is the quiet one. A pgx error can never satisfy err.(*mysql.MySQLError), so it returns false for every conflict and callers take the "not a duplicate" branch — no error, just wrong behaviour.

Change

• Add PostgreSQLDialect implementing SQLDialect, plus NewPostgreSQLDialect().
• Replace the hardcoded call with newStorageDialect(driverName), which rejects unknown drivers with the same message initDBDriver already uses rather than silently defaulting to one engine.

Implementation notes:

• STRING_AGG requires an explicit separator, unlike GROUP_CONCAT which defaults to ,. An empty separator falls back to , so the SQL keeps parsing and keeps MySQL's behaviour. A single-argument STRING_AGG does not parse; there is a test for this.
• Separators are single-quoted, since PostgreSQL reads a double-quoted token as an identifier rather than a string literal.
• IsDuplicateError uses errors.As because the pgx stdlib driver wraps *pgconn.PgError before it surfaces through database/sql. Tested with wrapped, bare, unrelated-SQLSTATE, non-PG, and nil errors.
• The SQLSTATE is a local pgUniqueViolation constant rather than adding a dependency on jackc/pgerrcode for one value.

What this does not fix

This does not make the PostgreSQL overlay work, and it is not intended to. Still outstanding from #13956:

• Item 1 — the storage call sites still build with squirrel's default ? placeholders instead of the $1 form the dialect in client_manager/dialect.go already defines.
• Item 2 — identifier casing.
• The Concat call sites in job_store.go:220 and run_store.go:270,283,298 pass double-quoted SQL string literals (`"["`, `"]"`) which PostgreSQL reads as identifiers. That is caller-side and left alone here deliberately, rather than writing MySQL-only syntax into a new PostgreSQL dialect.

Those need the design decision discussed in #13956 (comment). This PR removes one independent layer: the stores now receive a dialect matching the engine they are talking to.

Testing

10 new tests in db_test.go covering every PostgreSQLDialect method, both separator branches, both Upsert overwrite branches, and five IsDuplicateError cases. Plus a regression test asserting MySQLDialect.IsDuplicateError returns false for a PostgreSQL error, which is the bug being fixed.

ok  github.com/kubeflow/pipelines/backend/src/apiserver/storage         4.360s
ok  github.com/kubeflow/pipelines/backend/src/apiserver/client_manager  4.804s

Note for reproducing locally: the storage package needs CGO_ENABLED=1, since SQLiteDialect.IsDuplicateError uses mattn/go-sqlite3, which only exposes sqlite3.Error and sqlite3.ErrConstraint under cgo.

I have not run this against a live PostgreSQL instance — the remaining items above still prevent ml-pipeline from reaching Ready, so there is nothing end-to-end to verify against yet.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hbelmiro for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• backend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[google-oss-prow]

Hi @hsinhoyeh. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.