Port upstream PR #359: entity_tags dialect DDL test coverage + driver-matrix tests

[joestump-agent]

Ports upstream joestump#359, implementing joestump#307, adapted to this fork's existing structure.

Gap analysis (what the fork already had)

• MySQL/MariaDB dialect DDL in internal/database/entity_tags.go: entityTagsMySQL with BIGINT AUTO_INCREMENT, table-level FK constraints, single-statement ExecContext (works with the default MySQL DSN without multiStatements=true), and index creation guarded by an information_schema.statistics lookup (mysqlIndexExists) since MySQL 8 lacks CREATE INDEX IF NOT EXISTS. Functionally equivalent to upstream's inline-KEY approach.
• Postgres dialect parity: BIGSERIAL PRIMARY KEY, TIMESTAMPTZ, CREATE INDEX IF NOT EXISTS — already matches upstream.
• go.mod: lib/pq and go-sql-driver/mysql were already direct requires. No go.mod changes needed.
• Partial tests: SQLite create/idempotency test, MySQL DDL invariant test, DriverName dialect-detection test.

What was ported

• TestEntityTagsDDLDialects: per-dialect DDL token unit test, now covering Postgres and SQLite (fork previously only asserted MySQL invariants), including reject-tokens so dialect syntax can't leak across dialects.
• TestCreateEntityTagsTable_UnknownDriverFallsBackToSQLite: unknown drivers take the SQLite path, matching driverToStdlib.
• TestCreateEntityTagsTable_Matrix: driver-matrix integration test — SQLite in-memory always; Postgres/MariaDB gated behind SPOTTER_TEST_POSTGRES_DSN / SPOTTER_TEST_MYSQL_DSN with graceful skips (also skips if the DSN is set but the server is unreachable). Proves idempotency across two migration runs, insert/select round-trip, the (tag_id, entity_type, entity_id) unique constraint, and column types via PRAGMA table_info / information_schema.columns. Fork extension: the MySQL sub-test also verifies both indexes exist via the fork's mysqlIndexExists guard.
• MySQL DDL parity fixes from upstream: created_at is now DATETIME instead of TIMESTAMP (avoids the 2038 range limit and MySQL's implicit TIMESTAMP auto-init/auto-update behavior) and ENGINE=InnoDB is pinned explicitly (FK + ON DELETE CASCADE require InnoDB). Existing deployments are unaffected — the table is only created with IF NOT EXISTS.

Preserved fork-specific behavior: the mysqlIndexExists information_schema guard, single-Exec Postgres/SQLite DDL, driverPostgres constant usage, testify style, and the fork's MariaDB startup fixes (#21).

Defects found while porting

1. Upstream test false-positive risk (fixed): a naive reject-token like TIMESTAMP, for the MySQL dialect matches the substring inside CURRENT_TIMESTAMP,; the ported test rejects created_at TIMESTAMP instead. (Upstream's own AUTOINCREMENT reject-token is safe only because the underscore in AUTO_INCREMENT breaks the substring — the ported test keeps explicit per-dialect reject lists.)
2. Destructive test cleanup (documented): upstream's live-server helpers DROP TABLE IF EXISTS entity_tags/tags/users in the target database. Retained (needed for clean re-runs) but now carries an explicit warning to point the DSNs at throwaway databases only.
3. Noted, not changed: openLiveDB skips (rather than fails) when a configured DSN is unreachable — intentional per upstream so a stale env var doesn't break local runs, but it can mask CI misconfiguration.
4. No SQL injection in test helpers: all identifiers are hardcoded literals and the mysqlIndexExists guard uses parameterized queries.

Test results

• make generate — clean
• go build ./... — clean
• go vet ./... — clean
• go test ./internal/database/... — PASS (matrix Postgres/MySQL sub-tests SKIP gracefully, no servers in CI)
• go test ./... — all packages PASS

🤖 Generated with Claude Code

https://claude.ai/code/session_019Xyerp7w1SH5vG48z93DxN

---

Generated by Claude Code

[joestump-agent]

Adversarial review of the port of upstream joestump#359 (issue joestump#307). Verdict: APPROVE (posted as COMMENT because the review account authored the PR and GitHub forbids self-approval). No blocking findings.

Verified mechanically on the branch: make generate, go build ./..., go vet ./..., gofmt -l (clean), go test ./internal/database/... -run EntityTags -v (all pass; postgres/mysql matrix subtests skip cleanly), and full go test ./... green. Also probed the stale-DSN path with unreachable DSNs — openLiveDB skips gracefully on ping failure rather than failing.

Findings (all NON-BLOCKING):

1. NON-BLOCKING — internal/database/entity_tags.go:122 — the TIMESTAMP→DATETIME change only affects fresh databases. CREATE TABLE IF NOT EXISTS never migrates, so existing MySQL deployments keep a TIMESTAMP column (UTC-normalized, 2038-limited) while new installs get DATETIME (session-timezone-literal). This is benign today: I grepped every entity_tags code path — created_at is never SELECTed, UPDATEd, or ORDERed on (inserts in internal/tags/upsert.go omit it and rely on the column DEFAULT, which DATETIME DEFAULT CURRENT_TIMESTAMP supports on MySQL >= 5.6.5/MariaDB >= 10.0.1; the old DDL had no implicit ON UPDATE either, since an explicit DEFAULT suppresses it). But if a future feature starts reading created_at, the fleet will have mixed type/timezone semantics. Worth a one-line note in the DDL comment (or a follow-up ALTER migration issue) so this doesn't surprise anyone later.

2. NON-BLOCKING — internal/database/entity_tags_test.go:96-107 — token assertions are exact-substring, case- and spacing-sensitive. I checked the dangerous collision cases and they are actually sound as written: reject "AUTOINCREMENT" cannot substring-match "AUTO_INCREMENT" (underscore), reject "created_at TIMESTAMP" cannot match "DEFAULT CURRENT_TIMESTAMP" (prefixed column name scopes it). Remaining brittleness is minor: a regression to lowercase created_at timestamp or double-spaced tokens would false-pass the reject list, and a legitimate whitespace refactor (e.g. ENGINE = InnoDB, reordered DEFAULT CURRENT_TIMESTAMP NOT NULL) would false-fail the want list. Acceptable for a DDL-invariant test; just be aware these pins are stylistic as well as semantic.

3. NON-BLOCKING — internal/database/entity_tags_test.go:186-231, 250-262 — the live-DB subtests DROP users/tags/entity_tags gated only on presence of SPOTTER_TEST_POSTGRES_DSN / SPOTTER_TEST_MYSQL_DSN. Gating is effectively airtight in practice: the var names are project-specific, nothing in .github/ or the Makefile sets them, and unreachable servers skip instead of fail. If you want belt-and-suspenders against someone exporting these globally in a shell profile pointed at a real DB, a second opt-in var (e.g. SPOTTER_TEST_DESTRUCTIVE=1) would close it — the in-test WARNING comment is a reasonable substitute.

4. NON-BLOCKING (observation) — internal/database/entity_tags_test.go:145 — the new test cements the silent unknown-driver→SQLite fallback in CreateEntityTagsTable. This matches the pre-existing driverToStdlib contract in internal/database/db.go:67 (the whole app treats unknown drivers as sqlite3), and a genuinely wrong driver would fail loudly anyway (SQLite AUTOINCREMENT syntax is invalid on the other servers), so the fallback cannot silently corrupt a real database. Fine to test-pin it; erroring instead would be a separate, app-wide design change.

Probed without success: substring collisions in the reject-token logic, semicolon leakage into the single-statement MySQL DDL (ENGINE=InnoDB suffix keeps the pre-existing NotContains-";" assertion honest), shared-cache in-memory SQLite name collisions between tests, subtest ordering/cleanup dependencies (drop order respects FKs; each live subtest resets its own state), and information_schema case-sensitivity in the column assertions (scans are positional, values lowercased).

---

Generated by Claude Code