Skip to content

Add drop="if_exists" support to todb()#695

Merged
juarezr merged 1 commit into
petl-developers:masterfrom
akashmalbari:666-drop-table-if-exists
Jun 1, 2026
Merged

Add drop="if_exists" support to todb()#695
juarezr merged 1 commit into
petl-developers:masterfrom
akashmalbari:666-drop-table-if-exists

Conversation

@akashmalbari

Copy link
Copy Markdown
Contributor

Closes #666.

This PR has the objective of adding an idempotent drop mode to todb() so ETL jobs can recreate target tables whether or not the table already exists.

Changes

  1. Added support for drop="if_exists" when using todb(..., create=True).
  2. Fixed the missing-table recreate case by allowing todb() to call drop_table(..., if_exists=True) before creating the table.
  3. Preserved the existing drop=True behavior, which still issues a normal DROP TABLE and raises if the table does not exist.
  4. Updated the database table drop helper to emit DROP TABLE IF EXISTS only for the new if_exists mode.
  5. Added regression tests for the new drop="if_exists" behavior.
  6. Documented the change in docs/changes.rst.

Testing

  • .venv/bin/python -m pytest petl/test/io/test_db.py petl/test/io/test_db_create.py

Result:

  • 12 passed, 2 skipped, 2 warnings

The warnings are existing sqlite generator cleanup warnings from test_fromdb.

Checklist

Use this checklist to ensure the quality of pull requests that include new code and/or make changes to existing code.

  • Source Code guidelines:
    • Includes unit tests
    • New functions have docstrings with examples that can be run with doctest
    • New functions are included in API docs
    • Docstrings include notes for any changes to API or behavior
    • All changes are documented in docs/changes.rst
  • Versioning and history tracking guidelines:
    • Using atomic commits whenever possible
    • Commits are reversible whenever possible
    • There are no incomplete changes in the pull request
    • There is no accidental garbage added to the source code
  • Testing guidelines:
    • Tested locally using tox / pytest
    • Rebased to master branch and tested before sending the PR
    • Automated testing passes (see CI)
    • Unit test coverage has not decreased (see Coveralls)
  • State of these changes is:
    • Just a proof of concept
    • Work in progress / Further changes needed
    • Ready to review
    • Ready to merge

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Add drop='if_exists' support to todb() for idempotent operations

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add drop='if_exists' mode to todb() for idempotent table recreation
• Allow table drops without raising errors if table missing
• Preserve existing drop=True behavior with normal DROP TABLE
• Add comprehensive tests and documentation for new feature
Diagram
flowchart LR
  A["todb() with drop parameter"] -->|drop=True| B["DROP TABLE"]
  A -->|drop='if_exists'| C["DROP TABLE IF EXISTS"]
  A -->|drop=False| D["Skip drop"]
  B --> E["Create table"]
  C --> E
  D --> E
  E --> F["Load data"]

Loading

Grey Divider

File Changes

1. petl/io/db.py ✨ Enhancement +6/-3

Add if_exists parameter handling to todb()

• Updated drop parameter documentation to accept bool or 'if_exists' string
• Modified todb() logic to pass if_exists flag to drop_table() based on drop parameter value
• Conditional logic converts drop='if_exists' to if_exists=True for drop_table call

petl/io/db.py


2. petl/io/db_create.py ✨ Enhancement +7/-2

Add if_exists parameter to drop_table()

• Added if_exists parameter to drop_table() function signature with default False
• Updated docstring to document the new if_exists parameter
• Implemented conditional SQL generation: uses DROP TABLE IF EXISTS when if_exists=True,
 otherwise uses standard DROP TABLE

petl/io/db_create.py


3. petl/test/io/test_db.py 🧪 Tests +82/-0

Add comprehensive tests for drop modes

• Added LoggingConnection mock class to capture SQL statements for testing
• Added test_drop_table_if_exists_sql() to verify correct SQL generation for both drop modes
• Added helper function _record_todb_create_calls() to mock drop/create/load operations
• Added three test cases: test_todb_drop_false_skips_drop(),
 test_todb_drop_true_uses_plain_drop(), and test_todb_drop_if_exists_uses_if_exists() to verify
 correct behavior for each drop mode

petl/test/io/test_db.py


View more (2)
4. petl/test/io/test_db_create.py 🧪 Tests +15/-0

Add integration tests for if_exists mode

• Added test case for creating table with drop='if_exists' when table doesn't exist
• Added test case for recreating table with drop='if_exists' when table already exists
• Updated _setup_mysql() and _setup_generic() to clean up test table test_create_if_exists

petl/test/io/test_db_create.py


5. docs/changes.rst 📝 Documentation +3/-0

Document drop='if_exists' feature addition

• Added changelog entry documenting the new drop='if_exists' support in todb() function
• References issue #666

docs/changes.rst


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 1, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Grey Divider


Remediation recommended

1. Unvalidated drop string 🐞 Bug ≡ Correctness
Description
In todb(), any truthy drop value triggers a drop, but only the exact string 'if_exists'
enables idempotent dropping; other strings (including typos) silently fall back to a plain `DROP
TABLE` and can raise when the table is missing. This violates the documented parameter contract and
is not covered by tests for invalid drop values.
Code

petl/io/db.py[R352-355]

Evidence
todb() only checks drop truthiness and then sets if_exists via equality to 'if_exists', so
any other truthy value results in if_exists=False and a plain drop. The docstring claims only
bool or 'if_exists' are supported, and the new tests only exercise False, True, and
'if_exists', leaving the silent-fallback behavior unguarded.

petl/io/db.py[291-295]
petl/io/db.py[350-356]
petl/test/io/test_db.py[76-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`todb()` treats any truthy `drop` value as “drop the table”, but only special-cases the exact string `'if_exists'`. This means a typo like `drop='ifexist'` will still attempt a drop, but using a non-idempotent `DROP TABLE`, causing unexpected failures when the table is missing.

### Issue Context
The docstring states `drop` supports `bool or 'if_exists'`, but the implementation does not enforce this contract.

### Fix Focus Areas
- petl/io/db.py[350-360]
- petl/test/io/test_db.py[51-100]

### Suggested fix
1. Add explicit validation in `todb()`:
  - If `drop is False`: skip dropping.
  - If `drop is True`: drop with `if_exists=False`.
  - If `drop == 'if_exists'`: drop with `if_exists=True`.
  - Otherwise: raise `ArgumentError` (or `ValueError`) with a clear message listing allowed values.
2. Add a unit test asserting invalid values (e.g., `drop='ifexist'`, `drop='IF_EXISTS'`) raise the expected exception, to prevent regressions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 26760053583

Coverage increased (+0.03%) to 90.992%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 1 uncovered change across 1 file (57 of 58 lines covered, 98.28%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
petl/test/io/test_db.py 41 40 97.56%
Total (4 files) 58 57 98.28%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 14909
Covered Lines: 13566
Line Coverage: 90.99%
Coverage Strength: 0.91 hits per line

💛 - Coveralls

@akashmalbari

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look.

I noticed the Codacy Security Scan check is failing during the SARIF upload step with:

The CodeQL Action does not support uploading multiple SARIF runs with the same category.

This appears to happen after the Codacy analysis step completes, during github/codeql-action/upload-sarif@v4, so it looks workflow/config-related rather than related to this PR's Python code changes.

The local DB tests for this PR passed:

  • .venv/bin/python -m pytest petl/test/io/test_db.py petl/test/io/test_db_create.py

Result:

  • 12 passed, 2 skipped, 2 warnings

The warnings are existing sqlite generator cleanup warnings from test_fromdb.

Please let me know if you would like any changes to the PR itself.

@juarezr juarezr merged commit 677bd61 into petl-developers:master Jun 1, 2026
29 of 30 checks passed
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.

Feature Request: Drop table if exists

3 participants