feat(datafusion): Hierarchical namespace support #1977
Conversation
CTTY
left a comment
CTTY
left a comment
There was a problem hiding this comment.
Thanks for the contribution! The change LGTM, just one comment on the integration tests
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_child_namespace_crud() -> Result<()> { |
There was a problem hiding this comment.
We are trying to migrate off the rust datafusion integration tests to sqllogictests. Do you think we can rewrite this test in slt? Some examples can be found here: https://github.com/apache/iceberg-rust/tree/main/crates/sqllogictest/testdata/slts/df_test
There was a problem hiding this comment.
Cool, I haven't heard about that one. I changed it, please let me know which one you like better.
Side comment: I see the benefits of sqllogictest, but while writing the test, I was wondering how far sqllogictest gets you in interoperability testing SQL/Datafusion and Iceberg. In interoperability tests, you often verify fine points on each side against each other, which would not be visible through a test framework for one side only. As a result, you split tests between what you can do in the framework and what you can't do (see the PR). There may be a case for both "classic" and sqllogictest.
How the heck did that get here.
eickler
changed the title
eickler
changed the title
eickler
changed the title
2 participants
Which issue does this PR close?
What changes are included in this PR?
In the PR, instead of just listing the top-level namespaces in the IcebergCatalogProvider, the namespaces are hierarchically traversed and made available using a dot notation ("parent.child").
Are these changes tested?
The first commit contains only a) unit test cases that safeguard existing functionality and b) a unit test to demonstrate the gap with hierarchical namespaces (i.e., fails before the fix). Besides the unit tests, there is an additional integration test cases that demonstrates that CRUD on tables in child namespaces works with Datafusion using the dot notation.