Skip to content

fix: quote qualified Postgres table identifiers#498

Merged
yodakanohoshi merged 1 commit into
drt-hub:mainfrom
Photon101:fix/postgres-qualified-identifiers
May 14, 2026
Merged

fix: quote qualified Postgres table identifiers#498
yodakanohoshi merged 1 commit into
drt-hub:mainfrom
Photon101:fix/postgres-qualified-identifiers

Conversation

@Photon101

@Photon101 Photon101 commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Closes #442.

Summary

  • add helpers that split optional schema.table names and compose psycopg2.sql.Identifier with separate schema/relation parts
  • use the helpers across Postgres row count, replace, swap, finalize, insert, and upsert SQL paths
  • keep swap shadow/old suffixes on the relation name so marketing.email_events becomes marketing.email_events__drt_swap, not a single quoted identifier
  • add regression coverage for schema-qualified upsert, replace, swap, row-count, and replace error paths

Validation

  • uv run --extra dev --extra postgres python -m pytest tests/unit/test_postgres_destination.py tests/unit/test_dry_run_row_count.py tests/unit/test_dry_run_summary.py -v
  • uv run --extra dev --extra postgres python -m pytest --cov=drt.destinations.postgres --cov-report=term-missing tests/unit/test_postgres_destination.py
  • uv run --extra dev --extra postgres ruff check drt/destinations/postgres.py tests/unit/test_postgres_destination.py
  • uv run --extra dev --extra postgres mypy drt/destinations/postgres.py
  • git diff --check

@codecov

codecov Bot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.15385% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
drt/destinations/postgres.py 46.15% 14 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Photon101 Photon101 force-pushed the fix/postgres-qualified-identifiers branch from a10ff30 to c83da81 Compare May 13, 2026 01:45
@Photon101

Copy link
Copy Markdown
Collaborator Author

Updated in c83da81 after the initial Codecov report. I added coverage for the schema-qualified helper branches and replace-mode error paths; the latest codecov/patch status check is now green.

@yodakanohoshi yodakanohoshi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up on this one, @Photon101! 🙌

This is the right finishing touch for #442 — the helper trio
(_split_qualified / _qualified_ident / _with_relation_suffix)
cleans up every call site, and you caught the subtle bit that
ALTER TABLE ... RENAME TO only accepts a bare relation on the RHS.
Test coverage across upsert / replace / swap / row_count is exactly
what was asked for.

I'll add a CHANGELOG entry post-merge — no action needed from your side.

The MySQL / ClickHouse audit listed in #442's "Proposed Approach" is
out of scope for this PR and works well as a separate follow-up issue.
If you'd be interested in tackling it, I'd be glad to open one with
the scope written up — entirely no pressure, this PR stands on its own.

Thanks again for the contribution!

@yodakanohoshi yodakanohoshi merged commit bf755dc into drt-hub:main May 14, 2026
6 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Postgres destination: switch raw SQL f-strings to psycopg2.sql.Identifier for safe quoting

2 participants