feat(sqllogictest): support dynamic catalog configuration #1943
Conversation
|
The This is an intermittent infrastructure issue with GitHub Actions, not related to the code changes in this PR. Re-running the failed job should resolve it. |
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @AndreaBozzo for this pr, catalog config should be engine specific, not globally.
crates/sqllogictest/src/schedule.rs
Outdated
|
|
||
| /// Configuration for a catalog in the schedule file. | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct CatalogConfig { |
There was a problem hiding this comment.
Catalog config should be engine specific, e.g., it should be part of an engine, rather in a common section.
|
Sorry, i should have figured that out. I'll work on It and amend the commit when done! |
This PR implements issue apache#1780 by allowing each engine in the sqllogictest framework to configure its own catalog. Changes: - Remove global [catalog] section from schedule parsing - Each engine now creates its own catalog based on engine-specific config - DataFusionEngine reads 'catalog_type' and 'catalog_properties' from config - Default catalog type is 'memory' with a temp warehouse for testing - Support for all catalog types via iceberg-catalog-loader (rest, glue, hms, sql, s3tables) Example configuration: ```toml [engines] df = { type = "datafusion", catalog_type = "rest", catalog_properties = { uri = "http://localhost:8181" } } ``` Closes apache#1780
AndreaBozzo
force-pushed
the
feat/sqllogictest-dynamic-catalog
branch
from
7e8d26f to
4239a20
Compare
|
I've updated the PR to make the catalog configuration engine-specific as suggested. I considered adding explicit error handling for Let me know if you'd prefer that change and thank you for your patience |
2 participants
Which issue does this PR close?
Closes #1780
What changes are included in this PR?
This PR adds support for configuring the catalog type dynamically per engine in sqllogictest schedule files.
Changes:
catalog_typeandcatalog_propertiesfields to engine configurationmemorycatalog (default) and all catalog types fromiceberg-catalog-loader(rest, glue, hms, sql, s3tables)iceberg-catalog-loaderdependency to the workspaceExample configuration:
If no
catalog_typeis specified, defaults to memory catalog for backward compatibility.Are these changes tested?
Yes, unit tests added for:
Note: pr body updated after maintainer's review and feedbacks