fix: MemoryCatalog to return absolute NamespaceIdents #1970
Conversation
|
Note: I am currently trying to fix the Datafusion integration to work with nested namespaces. As part of unit testing using MemoryCatalog, I encountered this small issue. I read in the discussions that you prefer smaller PRs over larger ones, so I took the liberty of submitting it like this. |
CTTY
left a comment
CTTY
left a comment
There was a problem hiding this comment.
Thanks for the fix! Just a minor comment
| .map(|name| { | ||
| let mut names = parent_namespace_ident.iter().cloned().collect::<Vec<_>>(); | ||
| names.push(name.to_string()); | ||
| NamespaceIdent::from_vec(names).unwrap() |
There was a problem hiding this comment.
We should propgate the error rather than unwrapping it here
There was a problem hiding this comment.
Ah sorry, I have only started with Rust. I thought unwrap() would be reasonable here since names is guaranteed to have at least one element and the result can only be Err if names has no elements.
I could terminate on the first error and return the error. Is that better?
3 participants
Which issue does this PR close?
What changes are included in this PR?
The change makes
list_namespaces(parent)return an absolute namespace identifier instead of a relative.Are these changes tested?
The associated unit tests are updated with the same behaviour as SqlCatalog.