CXH-2381: document Db2 setup, correct the supported-database list, and fix the help link - #150
Conversation
Add a "Writing a Db2 spec" section to docs/db2.md (wrap every column reference in string(), alias columns to double-quoted lowercase), notes that account provisioning is unavailable and group principals can't be represented, and a "Running the Db2 tests" section with make test-db2 / make vet-db2 and the raw CGO_CFLAGS/CGO_LDFLAGS + library-path invocation. Add test-db2 and vet-db2 Makefile targets scoping the CGO flags to the recipe. Reconcile the engine lists (README, docs/docs-info.md, test/README.md) so they agree with the dispatch switch in pkg/database/database.go, and drop the unfilled template sentence from docs-info.md. Point the connector help URL at the live docs page (/docs/baton/baton-sql); the old /docs/baton/sql returns 404. Bump the ci.yaml sync-test action to @v4 and gitignore the root baton-sql build output.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Connector PR Review: CXH-2381: document Db2 setup, correct the supported-database list, and fix the help linkBlocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0 Review SummaryScanned the full PR diff — docs, the Security IssuesNone found. No secrets, injection, or credential-logging changes; the new error path emits a fixed message with no driver detail. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
…h check Add WordPress (MySQL-based) back to the README engine list so it agrees with docs/docs-info.md, which still ships it. Pass bad-credentials: DB_PASSWORD=invalid to sync-test@v4 so the new auth-error check actually runs; without it the step skips (DB_PASSWORD is not a BATON_* var, so nothing gets invalidated).
- db2.md: reframe the "Writing a Db2 spec" intro. The string()-wrapping and lowercase-alias patterns are not Db2-only; Oracle folds unquoted identifiers and Redshift needs string() in CEL concatenations. Db2 just needs both everywhere. - db2.md: correct the group-provisioning note. grantableTo is spec-driven and is not filtered by engine, so a spec that declares group still advertises it as grantable; the group grant is dropped at ingest, not at grantableTo. Split the two into separate statements. - docs-info.md: bring the "Example DSN formats" and "Configuration Examples" lists in line with the supported-engines list (SAP HANA, Vertica, Amazon Redshift, IBM DB2). - README.md: drop the hand-maintained engine enumeration in Key Features that had drifted from the Supported Database Engines list on WordPress; link to that list instead.
Replace os.Exit(1) in main.go with exit.LogExit(err) so an auth failure exits with the mapped gRPC status code (Unauthenticated/PermissionDenied) instead of a bare 1, which lets the CI sync-test auth-error check actually assert.
exit.LogExit alone made the CI bad-credentials check hard-fail: Validate wrapped the ping error with plain fmt.Errorf, so exit mapped it to Unknown(2) while auth-error.sh expects Unauthenticated(16)/PermissionDenied(7). Add database.AuthError: SQLSTATE class 28 (Postgres/Redshift/Vertica/etc.) and MySQL 1045 map to codes.Unauthenticated. Verified live: bad postgres creds now exit 16, good creds pass Validate.
| // AuthError returns an Unauthenticated gRPC status when err is a database | ||
| // authentication/authorization failure, or nil otherwise. SQLSTATE class 28 | ||
| // ("invalid authorization") is the ANSI code drivers report on bad credentials | ||
| // (Postgres/Redshift/Vertica/etc. surface it via SQLState()); MySQL is the | ||
| // exception, reporting error 1045 with no SQLSTATE. | ||
| func AuthError(err error) error { | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| var sqlState interface{ SQLState() string } | ||
| if errors.As(err, &sqlState) && strings.HasPrefix(sqlState.SQLState(), "28") { | ||
| return status.Error(codes.Unauthenticated, "database authentication failed") | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: the SQLState() interface probe only matches pgx — Vertica is named in the comment but doesn't qualify. vertica-sql-go@v1.3.6 declares SQLState as a struct field on VError (its only method is Error()), so errors.As against interface{ SQLState() string } never matches it; same for SQL Server (mssql.Error exposes SQLErrorState() uint8, not SQLState()), Oracle and HANA. Net effect is that only Postgres/Redshift and MySQL map to Unauthenticated — worth correcting the comment to say so, and optionally adding *vertigo.VError (SQLState 28000) / mssql.Error (18456) / go-ora ORA-01017 cases so the other engines return 16 instead of 2.
| if authErr := database.AuthError(err); authErr != nil { | ||
| return nil, authErr | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: this early return discards both the database name and the driver's message, so a multi-database config that fails auth on one connection logs only database authentication failed with no way to tell which DSN or why. Consider carrying the context through, e.g. return nil, status.Errorf(codes.Unauthenticated, "database %q authentication failed: %v", name, err) — the exit code stays 16 while the operator keeps the diagnostic detail.
4 participants
Documents how to set up the IBM Db2 database engine for the SQL connector, corrects the list of supported databases so it matches what the connector actually supports, and fixes the connector's help link, which pointed at a dead page.