-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(catalog): Cutover deprecated APIs to use session, catalog, table abstractions [3/3] #3830
Conversation
e4725d7
to
c47b29e
Compare
f1a2a5a
to
87810f1
Compare
7165ed8
to
470d238
Compare
Looks like there's a lot of merge conflicts. Could you merge from main first? |
91957d9
to
1a60f1d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3830 +/- ##
==========================================
+ Coverage 77.92% 78.06% +0.14%
==========================================
Files 763 760 -3
Lines 95766 95865 +99
==========================================
+ Hits 74626 74840 +214
+ Misses 21140 21025 -115
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Am I correct to think that we're working on getting qualified table support in SQL as well, and that we will eventually eliminate SQLCatalog
and only use the session catalog everywhere and for all our frontends?
} | ||
} | ||
pub curr_catalog: Option<String>, | ||
pub curr_namespace: Option<Vec<String>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we're moving away from the term "namespace" and into "qualifier" in some places but not in others. How are you thinking about the difference between these two terms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Here's the PR for qualified identifiers in SQL feat(sql): adds session sql for leveraging attached catalogs #3860
- Qualifies are the first parts of an identifier
- Namespaces are a path within a catalog.
The qualifier can contain a catalog, so it's not exactly a namespace. Namespace is like a dir in a volume if the volume is catalog.
This PR swaps existing functionality to be backed by the new session, catalog, and table APIs.
Changes
daft.register_python_catalog -> daft.attach_catalog
daft.unregister_catalog -> daft.detach_catalog
daft.read_table -> daft.read_table
(via session)daft.register_table -> daft.attach_table
Context