Skip to content

test: PostgreSQL plugin migration — Phase 0 baseline test infrastructure - #577

Draft
aesslinger wants to merge 20 commits into
TabularisDB:mainfrom
aesslinger:postgres-plugin-migration
Draft

test: PostgreSQL plugin migration — Phase 0 baseline test infrastructure#577
aesslinger wants to merge 20 commits into
TabularisDB:mainfrom
aesslinger:postgres-plugin-migration

Conversation

@aesslinger

Copy link
Copy Markdown
Contributor

Ref #16

⚠️ Draft — Checkpoint CP-2

This is a long-lived branch for the PostgreSQL plugin migration initiative.
It is currently at Phase 0 (baseline test infrastructure) and will
continue through Phase 1 (plugin build) within this same PR.

Status: Phase 0 complete — requesting team review before proceeding to Phase 1.

What This PR Contains Today

RPC Driver Extensions (prerequisite — also in PR #576)

  • BLOB method forwarding (save_blob_to_file, fetch_blob_as_data_url)
  • Materialized view method forwarding (4 methods)
  • type_mappings manifest field for synchronous type resolution

Phase 0: Integration Test Baseline

  • 93 tests (89 new + 4 un-ignored existing) covering every public
    PostgreSQL driver method
  • 17 golden snapshot files recording exact driver output as the parity contract
  • Separate CI workflow (pg-integration.yml) with PostgreSQL 16 service
  • Idempotent seed script (2 databases, 3 schemas, all PG types)
  • Golden file capture/compare utilities

Planning Documentation

  • .github/planning/postgres-plugin-migration.md — TDD approach with multi-db from day 1
  • .github/planning/postgres-plugin/ — Per-phase detailed docs (prerequisites, Phase 0-3)

What Comes Next (Phase 1 — within this PR)

Once CP-2 is acknowledged, Phase 1 begins: building postgres-plugin using
strict TDD. The 89 parity tests start RED against the plugin and turn GREEN
sprint by sprint. Progress is tracked as an objective metric (0/89 → 89/89).

See .github/planning/postgres-plugin/02-phase-1-plugin-build.md for the
full sprint breakdown.

How to Validate Locally

# Start PG 16
docker run -d --name pg-parity -p 54320:5432 \
  -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=password \
  -e POSTGRES_DB=testdb postgres:16

# Seed
bash tests/fixtures/seed_postgres.sh

# Run all 89 tests
cd src-tauri && cargo test --test postgres_integration -- --include-ignored --test-threads=1

Depends On

Extend the RpcDriver to forward save_blob_to_file and
fetch_blob_as_data_url to plugin processes via JSON-RPC.

Plugins that implement these methods can now handle binary data
export/preview. Plugins that do not implement them receive a graceful
fallback via is_method_not_found (same pattern as routines, triggers).
Extend the RpcDriver to forward get_materialized_views,
get_materialized_view_columns, get_materialized_view_definition,
and refresh_materialized_view to plugin processes via JSON-RPC.

Plugins that declare materialized_views capability can now serve
these queries. Plugins without support receive graceful fallbacks
via is_method_not_found (empty vec or unsupported error).
Add an optional type_mappings field to PluginManifest and ConfigManifest
that maps generic inferred types (e.g. DATETIME, JSON) to driver-specific
types (e.g. TIMESTAMP, JSONB).

The RpcDriver now overrides map_inferred_type to consult these static
mappings at lookup time. This avoids the need for an async RPC call in a
synchronous trait method.

Built-in drivers continue to use their direct trait overrides and declare
empty mappings. Existing plugins without type_mappings are unaffected
(serde default is an empty map, passthrough behavior is preserved).
- Add separate CI workflow (pg-integration.yml) with PG 16 service
- Add seed script (postgres_seed.sql + seed_postgres.sh) for test schemas
- Add integration test harness (postgres_integration/) with 22 tests:
  - schema_discovery: 4 tests (get_schemas, get_databases, get_tables)
  - column_metadata: 6 tests (PK, nullable, types, max_length, enum)
  - indexes: 4 tests (btree, unique, composite, primary key)
  - foreign_keys: 4 tests (basic, composite table, cross-schema, empty)
  - query_execution: 6 tests (basic SELECT, pagination, all types,
    null handling, DML affected_rows, batch session state)
- All tests use #[ignore] and require PG on port 54320
- Seed creates test_schema + other_schema + secondary database
- CI job is separate from main test job (temporary for plugin migration)
…database tests (WIP)

Adds remaining test modules for Phase 0 baseline. Some modules have
compilation errors due to API signature mismatches that need fixing:
- routines.rs: RoutineInfo has no specific_name field
- crud.rs: update_record takes (pk_map, col_name, value) not (data, pk_map)
- routines.rs: drop_routine and get_routine_definition signature differences

These will be fixed in the next commit.
Fix compilation errors from incorrect function signature assumptions:
- routines.rs: use RoutineInfo.routine_type (String, not Option)
- routines.rs: get_routine_definition takes routine_type param
- routines.rs: drop_routine takes routine_type not arg signature
- crud.rs: update_record takes (&pk_map, col_name, value) per-column
- crud.rs: delete_record takes &HashMap (borrow, not owned)
- triggers.rs: get_trigger_definition requires table_name param

All 56 integration tests now compile successfully.
All test modules compile and cover the full PostgreSQL driver API:
- schema_discovery: 4 tests
- column_metadata: 6 tests
- indexes: 4 tests
- foreign_keys: 4 tests
- views: 6 tests
- materialized_views: 4 tests
- routines: 6 tests
- triggers: 4 tests
- crud: 8 tests
- query_execution: 6 tests
- multi_database: 7 tests
- ddl_generation: 7 tests
- explain: 3 tests
- blob: 3 tests

Total: 72 integration tests covering every public method of the
PostgreSQL driver. All use #[ignore] and require PG on port 54320.
CI workflow (pg-integration.yml) runs them with --include-ignored.
- Fix null handling test: exclude col_uuid (has DEFAULT gen_random_uuid())
- Fix character_max_length test: accept None (driver doesn't populate it)
- Fix enum type assertion: driver returns enum('val1','val2') format
- Fix MV definition test: handle pre-existing driver bug gracefully
- Fix blob insert test: use BLOB wire format instead of hex string
- Fix trigger test: cleanup at start (idempotent against prior failed runs)
- Fix alter_view test: cleanup at start (same reason)

All 72 tests pass with --test-threads=1 against PostgreSQL 16.
Two tests were weakened in the prior commit to 'accept either behavior' —
this violates TDD principles. Tests must assert the EXACT behavior:

- character_max_length: Assert None explicitly (known driver limitation).
  The plugin must return None too. If the driver is fixed later, this test
  will correctly fail — prompting both test and plugin updates.

- MV definition: Assert the error explicitly (known driver bug on PG 16).
  The plugin must produce the same error. If the bug is fixed upstream,
  this test will correctly fail — signaling the spec has changed.

Principle: Tests ARE the specification. A passing test means the behavior
is correct. We never weaken a test to accommodate — we assert what IS.
Golden files record the exact output of driver methods against the seeded
test database. They serve as the parity contract for Phase 1 — the plugin
must produce output matching these files byte-for-byte.

Adds:
- golden_utils.rs: write_golden() and assert_golden() helpers
- golden.rs: 17 capture/compare tests covering schemas, tables, columns,
  indexes, FKs, views, MVs, routines, triggers, queries, explain, multi-db
- golden/ directory: 17 committed JSON snapshots

To regenerate golden files after driver changes:
  REGENERATE_GOLDEN=1 cargo test --test postgres_integration golden -- --include-ignored --test-threads=1

Total test count: 89 (72 integration + 17 golden)
Remove #[ignore] from the 4 existing PG integration tests:
- test_postgres_integration_flow
- test_postgres_batch_preserves_temp_table_and_transaction
- test_postgres_affected_rows_reported_correctly
- test_postgres_foreign_keys_via_pg_catalog

These tests soft-skip (eprintln + return) if PG isn't available, so they
won't break the main CI that doesn't have a PG service. They WILL run in
our pg-integration.yml workflow and in the standard cargo test flow when
a local PG is available.

MySQL tests remain #[ignore] (no MySQL in CI).

Total tests now running against PG: 89 (new suite) + 4 (existing) = 93
Includes:
- postgres-plugin-migration.md (original phased plan)
- postgres-plugin-migration-alt.md (TDD approach with multi-db from day 1)
- postgres-plugin/ directory (per-phase detailed docs)
- sqlite-improvements.md (SQLite driver audit)
- .markdownlint.json config for planning docs
Fixes critical and high issues from deep code review:

Critical:
- Use deterministic UUID in seed (fixed value, not gen_random_uuid())
  so golden files produce identical output across environments
- EXPLAIN golden test writes for documentation only (no exact assert) —
  plan costs/widths are volatile across PG versions and table stats

High:
- CRUD tests now clean up inserted rows (no state accumulation)
- Restore #[ignore] on existing integration_tests.rs PG tests
  (avoids 20s timeout penalty on normal cargo test; CI uses --include-ignored)
- CI workflow: add apt-get update before postgresql-client install
- Remove unused TABULARIS_TEST_PG env var from CI

Low:
- Golden files now include trailing newline (POSIX compliance)
- Remove unnecessary #[allow(dead_code)] on pg_params_secondary
- Rename plan docs (alt plan is now primary)
Extend the RpcDriver to forward save_blob_to_file and
fetch_blob_as_data_url to plugin processes via JSON-RPC.

Plugins that implement these methods can now handle binary data
export/preview. Plugins that do not implement them receive a graceful
fallback via is_method_not_found (same pattern as routines, triggers).
Extend the RpcDriver to forward get_materialized_views,
get_materialized_view_columns, get_materialized_view_definition,
and refresh_materialized_view to plugin processes via JSON-RPC.

Plugins that declare materialized_views capability can now serve
these queries. Plugins without support receive graceful fallbacks
via is_method_not_found (empty vec or unsupported error).
Add an optional type_mappings field to PluginManifest and ConfigManifest
that maps generic inferred types (e.g. DATETIME, JSON) to driver-specific
types (e.g. TIMESTAMP, JSONB).

The RpcDriver now overrides map_inferred_type to consult these static
mappings at lookup time. This avoids the need for an async RPC call in a
synchronous trait method.

Built-in drivers continue to use their direct trait overrides and declare
empty mappings. Existing plugins without type_mappings are unaffected
(serde default is an empty map, passthrough behavior is preserved).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant