-
Notifications
You must be signed in to change notification settings - Fork 378
Fix: SqlCatalog
list_namespaces() should return only sub-namespaces
#1629
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
Merged
Fokko
merged 8 commits into
apache:main
from
alessandro-nori:issue_1627_sqlcatalog_list_namespaces
Mar 5, 2025
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
90cc650
fix SqlCatalog list_namespaces()
alessandro-nori 619f11e
fix linter errors
alessandro-nori 7523a90
SqlCatalog: fix tests using list_namespaces
alessandro-nori 58fcb40
fix InMemoryCatalog tests
alessandro-nori fac3343
extract len() out of loop
alessandro-nori d1d590a
refactor, address review
alessandro-nori 83a0c54
minor tests refactor
alessandro-nori 53df102
new test for fuzzy match namespaces
alessandro-nori File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
|
||
import os | ||
from pathlib import Path | ||
from typing import Any, Generator, List, cast | ||
from typing import Any, Generator, cast | ||
|
||
import pyarrow as pa | ||
import pytest | ||
|
@@ -1027,7 +1027,7 @@ def test_create_namespace_if_not_exists(catalog: SqlCatalog, database_name: str) | |
@pytest.mark.parametrize("namespace", [lazy_fixture("database_name"), lazy_fixture("hierarchical_namespace_name")]) | ||
def test_create_namespace(catalog: SqlCatalog, namespace: str) -> None: | ||
catalog.create_namespace(namespace) | ||
assert (Catalog.identifier_to_tuple(namespace)) in catalog.list_namespaces() | ||
assert (Catalog.identifier_to_tuple(namespace)[:1]) in catalog.list_namespaces() | ||
|
||
|
||
@pytest.mark.parametrize( | ||
|
@@ -1074,7 +1074,7 @@ def test_create_namespace_with_comment_and_location(catalog: SqlCatalog, namespa | |
} | ||
catalog.create_namespace(namespace=namespace, properties=test_properties) | ||
loaded_database_list = catalog.list_namespaces() | ||
assert Catalog.identifier_to_tuple(namespace) in loaded_database_list | ||
assert Catalog.identifier_to_tuple(namespace)[:1] in loaded_database_list | ||
properties = catalog.load_namespace_properties(namespace) | ||
assert properties["comment"] == "this is a test description" | ||
assert properties["location"] == test_location | ||
|
@@ -1135,17 +1135,42 @@ def test_namespace_exists(catalog: SqlCatalog) -> None: | |
lazy_fixture("catalog_sqlite"), | ||
], | ||
) | ||
@pytest.mark.parametrize("namespace_list", [lazy_fixture("database_list"), lazy_fixture("hierarchical_namespace_list")]) | ||
def test_list_namespaces(catalog: SqlCatalog, namespace_list: List[str]) -> None: | ||
def test_list_namespaces(catalog: SqlCatalog) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for adding this test! |
||
namespace_list = ["db", "db.ns1", "db.ns1.ns2", "db.ns2", "db2", "db2.ns1", "db%"] | ||
for namespace in namespace_list: | ||
catalog.create_namespace(namespace) | ||
# Test global list | ||
if not catalog._namespace_exists(namespace): | ||
catalog.create_namespace(namespace) | ||
|
||
ns_list = catalog.list_namespaces() | ||
for ns in [("db",), ("db%",), ("db2",)]: | ||
assert ns in ns_list | ||
|
||
ns_list = catalog.list_namespaces("db") | ||
assert sorted(ns_list) == [("db", "ns1"), ("db", "ns2")] | ||
|
||
ns_list = catalog.list_namespaces("db.ns1") | ||
assert sorted(ns_list) == [("db", "ns1", "ns2")] | ||
|
||
ns_list = catalog.list_namespaces("db.ns1.ns2") | ||
assert len(ns_list) == 0 | ||
|
||
Fokko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@pytest.mark.parametrize( | ||
"catalog", | ||
[ | ||
lazy_fixture("catalog_memory"), | ||
lazy_fixture("catalog_sqlite"), | ||
], | ||
) | ||
def test_list_namespaces_fuzzy_match(catalog: SqlCatalog) -> None: | ||
namespace_list = ["db.ns1", "db.ns1.ns2", "db.ns2", "db.ns1X.ns3", "db_.ns1.ns2", "db2.ns1.ns2"] | ||
for namespace in namespace_list: | ||
assert Catalog.identifier_to_tuple(namespace) in ns_list | ||
# Test individual namespace list | ||
assert len(one_namespace := catalog.list_namespaces(namespace)) == 1 | ||
assert Catalog.identifier_to_tuple(namespace) == one_namespace[0] | ||
if not catalog._namespace_exists(namespace): | ||
catalog.create_namespace(namespace) | ||
|
||
assert catalog.list_namespaces("db.ns1") == [("db", "ns1", "ns2")] | ||
|
||
assert catalog.list_namespaces("db_.ns1") == [("db_", "ns1", "ns2")] | ||
|
||
|
||
@pytest.mark.parametrize( | ||
|
@@ -1177,13 +1202,13 @@ def test_list_non_existing_namespaces(catalog: SqlCatalog) -> None: | |
def test_drop_namespace(catalog: SqlCatalog, table_schema_nested: Schema, table_identifier: Identifier) -> None: | ||
namespace = Catalog.namespace_from(table_identifier) | ||
catalog.create_namespace(namespace) | ||
assert namespace in catalog.list_namespaces() | ||
assert catalog._namespace_exists(namespace) | ||
catalog.create_table(table_identifier, table_schema_nested) | ||
with pytest.raises(NamespaceNotEmptyError): | ||
catalog.drop_namespace(namespace) | ||
catalog.drop_table(table_identifier) | ||
catalog.drop_namespace(namespace) | ||
assert namespace not in catalog.list_namespaces() | ||
assert not catalog._namespace_exists(namespace) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.