HansiChan commented on PR #7720:
URL: https://github.com/apache/paimon/pull/7720#issuecomment-4525996866
Thanks for the detailed review. I pushed a follow-up commit to address the
JDBC catalog comments:
- Added an explicit transaction context in `_DbApiConnection`; `execute()`
no longer commits each statement.
- Wrapped compound catalog operations in transactions, including
`create_table`, `drop_database`, `alter_table`, `rename_table`, and property
writes.
- `create_table` now rolls back metadata on failure and still cleans up the
table directory.
- `rename_table` now moves the table path before updating metadata, and
tries to move it back if the metadata update fails.
- Replaced naive placeholder substitution with conversion that skips quoted
SQL string literals.
- Popped MySQL/PostgreSQL connection kwargs such as `host`, `port`,
`database` / `dbname` before passing remaining props to the driver.
- Set SQLite `check_same_thread=False`.
- Added `JdbcCatalog.__enter__` / `__exit__`.
- Switched property inserts to `executemany()` and added tests for rollback,
rename failure, placeholder conversion, and context manager cleanup.
I also checked the current CI failures and they look unrelated to this JDBC
catalog change. I double-checked the PR diff: this PR only changes the JDBC
catalog implementation/docs/tests and does not touch the GCS, Tantivy, or mixed
e2e code paths.
The failed checks are:
- `lint-python (3.6.15)` fails in GCS file IO tests because the Python 3.6
job installs `pyarrow==6.0.1`, where `pyarrow.fs.GcsFileSystem` is not
available.
- `lint-python (3.10)` and `lint-python (3.11)` pass the normal PyPaimon
tests, including `pypaimon/tests/jdbc_catalog_test.py`, but fail later in the
mixed e2e Tantivy full-text index test with:
`AttributeError: 'tantivy.tantivy.Searcher' object has no attribute
'fast_field_values'`.
Local validation:
`PYTHONPATH=. python -m pytest pypaimon/tests/jdbc_catalog_test.py`
`python -m flake8 --config=dev/cfg.ini pypaimon/catalog/jdbc_catalog.py
pypaimon/tests/jdbc_catalog_test.py`
I will leave the unrelated CI failures for now unless maintainers prefer me
to handle them in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
