Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Postgres Plugin for dbt-duckdb to Use ATTACH Syntax and Improve Compatibility Gexar/update pgsql plugin #478 #480

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Gexar
Copy link

@Gexar Gexar commented Nov 21, 2024

Description:

This pull request updates the Postgres plugin for the dbt-duckdb adapter to align with the latest DuckDB PostgreSQL extension (ATTACH syntax) and introduces improvements to functionality, testing, and error handling.

Changes Made:

  1. Migration to ATTACH Syntax:

    • Replaced the deprecated CALL postgres_attach syntax with the new ATTACH command, returning the write-to-pg functionality.
    • Removed deprecated features like overwrite in favor of READ_ONLY.
  2. Enhanced Configuration:

    • Added support for configuring additional attach options and extension settings, ensuring compatibility with the latest DuckDB Postgres extension features.
  3. Improved Logging and Debugging:

    • Utilized AdapterLogger to provide clearer and more detailed error messages.
    • Enhanced traceability of plugin configuration and database attachment processes.
  4. Updated Unit Tests:

    • Refactored and extended test coverage to validate new functionality and edge cases:
      • Successful ATTACH with default and custom configurations.
      • Read/write operations in both read-only and read-write modes.
      • Handling of PostgreSQL arrays and error conditions for invalid configurations.
  5. Code Refactoring:

    • Improved code readability and maintainability by introducing helper methods (_set_extension_settings, _build_attach_statement).
    • Added type annotations for better developer experience and debugging.

Testing:

  • Verified all test cases pass successfully:
    • ✅ 8 tests executed covering all key scenarios.
    • 🔧 Addressed issues with outdated PostgreSQL array handling and error messages.
  • All changes validated against DuckDB version 1.1.3.

Impact:

  • Ensures compatibility with DuckDB’s latest PostgreSQL extension.
  • Improves user experience with better logging, clearer error messages, and broader configurability.
  • Streamlines the plugin's codebase for easier maintenance and future updates.

Checklist:

  • Updated postgres.py to support ATTACH syntax.
  • Updated test_postgres.py to reflect new features and configurations.

Related Issues:
Unfortunately, due to this issue the testing has to be done outside the VSCode dev container.
#478
closed previous PR due to a bad structuring of unit tests.

Please review the changes and let me know if additional updates or refinements are needed. Thank you for your time and consideration!🚀

@Gexar Gexar force-pushed the gexar/update_pgsql_plugin_478 branch 3 times, most recently from 58c229a to 5b953b3 Compare November 25, 2024 19:49
@jwills
Copy link
Collaborator

jwills commented Nov 26, 2024

@Gexar hey there-- sorry for the lag here, I'm traveling with family right now in Asia and am not at a computer as often as I am when I'm back home. I'm on-board with the changes here and am really just waiting for the tests to pass before doing a final review and merging it in, but it may just be next week before I have a large enough block of time to do it. Appreciate your understanding here.

@jwills
Copy link
Collaborator

jwills commented Nov 26, 2024

Looks like it's just some linting stuff-- I realize this sort of thing is hard to fix on a WSL box w/o the devcontainer to run it for you! https://github.com/duckdb/dbt-duckdb/actions/runs/12017675321/job/33522437209?pr=480

…tialization, and test assertions; gitignore for a wsl development workflow
@Gexar Gexar force-pushed the gexar/update_pgsql_plugin_478 branch from 5b953b3 to 5a342bb Compare November 26, 2024 09:25
@Gexar
Copy link
Author

Gexar commented Nov 26, 2024

@jwills Thank you for your prompt reply! No worries at all—I completely understand! I hope you’re having a wonderful time traveling with your family.

I’ve gone ahead and addressed the linting issues (apologies for missing them earlier!) and pushed the fixes. Everything should be good to go now, but please don’t hesitate to let me know if there’s anything else I can adjust or improve.

I really appreciate your time, especially while you’re traveling, and I’m grateful for your guidance and support on this. Looking forward to your review whenever you have the chance!

Safe travels, and enjoy your trip! 🚀

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.

2 participants