[core][flink] Support view operations in JDBC Catalog#6667
Conversation
- Add view methods to CachingCatalog for proper delegation - Add IT tests for JDBC Catalog view operations - Add SQLite JDBC dependency for tests
be21196 to
7ad7450
Compare
|
This pull request has had no activity for 90 days. If you'd like to keep it open, please push a new commit or leave a comment. Thanks for the contribution. |
JingsongLi
left a comment
There was a problem hiding this comment.
Nice feature adding view support to JDBC Catalog (+716 lines). Review:
-
Completeness: Implements all view operations (create, get, list, drop, rename, alter) - this is a complete feature.
-
CachingCatalog delegation: Adding view method delegation ensures views work correctly when the catalog is wrapped. Good.
-
JdbcUtils SQL: The view metadata is stored in JDBC tables alongside table metadata. Key questions:
- What's the schema for the view storage table? Is it created automatically (schema migration)?
- Is the view SQL text stored as-is, or is there any parsing/validation?
- How are cross-database view references handled?
-
Testing: 7 integration tests is good coverage for the basic operations. Consider adding:
- View referencing a non-existent table (should this fail at create time or query time?)
- Rename view across databases
- Concurrent view creation
-
Flink integration test: Good that there's a dedicated
JdbcCatalogViewITCasetesting end-to-end with Flink SQL. -
SQLite test dependency: Reasonable for unit tests, but ensure it doesn't leak into production classpath.
Looks well-structured. Would be good to confirm the DDL for the views table in the JDBC schema.
Purpose
Add view support for JDBC Catalog to enable CREATE VIEW, DROP VIEW, SHOW VIEWS and other view operations.
Changes
Tests
All 7 integration tests pass: