Skip to content

Commit

Permalink
Fix SqlCatalog unit tests broken from code update.
Browse files Browse the repository at this point in the history
  • Loading branch information
cccs-eric committed May 15, 2024
1 parent 744c742 commit 066f738
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 18 deletions.
11 changes: 6 additions & 5 deletions pyiceberg/catalog/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,12 @@ class IcebergNamespaceProperties(SqlCatalogBaseTable):
class SqlCatalog(MetastoreCatalog):
"""Implementation of a SQL based catalog.
In the JDBCCatalog implementation, a Namespace is composed of a list of strings separated by dots: 'ns1.ns2.ns3'.
And you can have as many levels as you want, but you need at least one.
In the `JDBCCatalog` implementation, a `Namespace` is composed of a list of strings separated by dots: `'ns1.ns2.ns3'`.
And you can have as many levels as you want, but you need at least one. The `SqlCatalog` honors the same convention.
In the JDBCCatalog implementation, a TableIdentifier is composed of an optional Namespace and a table name.
When a Namespace is present, the full name will be 'ns1.ns2.ns3.table'. A valid TableIdentifier could be 'name' (no namespace).
In the `JDBCCatalog` implementation, a `TableIdentifier` is composed of an optional `Namespace` and a table name.
When a `Namespace` is present, the full name will be `'ns1.ns2.ns3.table'`. A valid `TableIdentifier` could be `'name'` (no namespace).
The `SqlCatalog` has a different convention where a `TableIdentifier` requires a `Namespace`.
"""

def __init__(self, name: str, **properties: str):
Expand Down Expand Up @@ -548,7 +549,7 @@ def list_tables(self, namespace: Union[str, Identifier]) -> List[Identifier]:
"""
if namespace and not self._namespace_exists(namespace):
raise NoSuchNamespaceError(f"Namespace does not exist: {namespace}")

namespace = Catalog.namespace_to_string(namespace)
stmt = select(IcebergTables).where(IcebergTables.catalog_name == self.name, IcebergTables.table_namespace == namespace)
with Session(self.engine) as session:
Expand Down
45 changes: 32 additions & 13 deletions tests/catalog/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,19 +367,29 @@ def test_create_table_with_default_warehouse_location(
lazy_fixture('catalog_sqlite'),
],
)
@pytest.mark.parametrize(
"table_identifier",
[
lazy_fixture("random_table_identifier"),
lazy_fixture("random_hierarchical_identifier"),
lazy_fixture("random_table_identifier_with_catalog"),
],
)
def test_create_table_with_given_location_removes_trailing_slash(
warehouse: Path, catalog: SqlCatalog, table_schema_nested: Schema, random_identifier: Identifier
warehouse: Path, catalog: SqlCatalog, table_schema_nested: Schema, table_identifier: Identifier
) -> None:
database_name, table_name = random_identifier
location = f"file://{warehouse}/{database_name}.db/{table_name}-given"
catalog.create_namespace(database_name)
catalog.create_table(random_identifier, table_schema_nested, location=f"{location}/")
table = catalog.load_table(random_identifier)
assert table.identifier == (catalog.name,) + random_identifier
table_identifier_nocatalog = catalog.identifier_to_tuple_without_catalog(table_identifier)
namespace = Catalog.namespace_from(table_identifier_nocatalog)
table_name = Catalog.table_name_from(table_identifier_nocatalog)
location = f"file://{warehouse}/{catalog.name}.db/{table_name}-given"
catalog.create_namespace(namespace)
catalog.create_table(table_identifier, table_schema_nested, location=f"{location}/")
table = catalog.load_table(table_identifier)
assert table.identifier == (catalog.name,) + table_identifier_nocatalog
assert table.metadata_location.startswith(f"file://{warehouse}")
assert os.path.exists(table.metadata_location[len("file://") :])
assert table.location() == location
catalog.drop_table(random_identifier)
catalog.drop_table(table_identifier)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -1401,11 +1411,20 @@ def test_table_properties_raise_for_none_value(
lazy_fixture('catalog_sqlite'),
],
)
def test_table_exists(catalog: SqlCatalog, table_schema_simple: Schema, random_identifier: Identifier) -> None:
database_name, _table_name = random_identifier
catalog.create_namespace(database_name)
catalog.create_table(random_identifier, table_schema_simple, properties={"format-version": "2"})
existing_table = random_identifier
@pytest.mark.parametrize(
"table_identifier",
[
lazy_fixture("random_table_identifier"),
lazy_fixture("random_hierarchical_identifier"),
lazy_fixture("random_table_identifier_with_catalog"),
],
)
def test_table_exists(catalog: SqlCatalog, table_schema_simple: Schema, table_identifier: Identifier) -> None:
table_identifier_nocatalog = catalog.identifier_to_tuple_without_catalog(table_identifier)
namespace = Catalog.namespace_from(table_identifier_nocatalog)
catalog.create_namespace(namespace)
catalog.create_table(table_identifier, table_schema_simple, properties={"format-version": "2"})
existing_table = table_identifier
# Act and Assert for an existing table
assert catalog.table_exists(existing_table) is True

Expand Down

0 comments on commit 066f738

Please sign in to comment.