You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ke...@apache.org on 2025/06/14 18:16:34 UTC
(iceberg-python) branch main updated: change catalog default warehouse location to not use hive-style warehouse location (#2059)
This is an automated email from the ASF dual-hosted git repository.
kevinjqliu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
The following commit(s) were added to refs/heads/main by this push:
new 41ff734a change catalog default warehouse location to not use hive-style warehouse location (#2059)
41ff734a is described below
commit 41ff734aa68beda54775a810599aae1552162beb
Author: Kevin Liu <ke...@users.noreply.github.com>
AuthorDate: Sat Jun 14 14:16:30 2025 -0400
change catalog default warehouse location to not use hive-style warehouse location (#2059)
<!--
Thanks for opening a pull request!
-->
<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
Closes #2052
# Rationale for this change
Aligns catalog behavior with the java reference implementation.
[HiveCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java#L697-L736),
[DynamoDbCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java#L185),
and
[GlueCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java#L291)
all use `.db` suffix in warehouse location
[JdbcCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java#L268),
[HadoopCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java#L233-L245),
and
[InMemoryCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/inmemory/InMemoryCatalog.java#L106-L117)
do not use `.db` suffix in warehouse location
# Are these changes tested?
Yes tests for sql catalog are modified to remove `.db`
# Are there any user-facing changes?
<!-- In the case of user-facing changes, please add the changelog label.
-->
---
pyiceberg/catalog/__init__.py | 14 ++++++++++++++
pyiceberg/catalog/dynamodb.py | 4 ++++
pyiceberg/catalog/glue.py | 4 ++++
pyiceberg/catalog/hive.py | 4 ++++
tests/catalog/test_sql.py | 30 +++++++++++++++---------------
tests/cli/test_console.py | 4 ++--
6 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py
index d7d4c9d0..81d3a34e 100644
--- a/pyiceberg/catalog/__init__.py
+++ b/pyiceberg/catalog/__init__.py
@@ -927,6 +927,20 @@ class MetastoreCatalog(Catalog, ABC):
return location.rstrip("/")
def _get_default_warehouse_location(self, database_name: str, table_name: str) -> str:
+ """Return the default warehouse location using the convention of `warehousePath/databaseName/tableName`."""
+ database_properties = self.load_namespace_properties(database_name)
+ if database_location := database_properties.get(LOCATION):
+ database_location = database_location.rstrip("/")
+ return f"{database_location}/{table_name}"
+
+ if warehouse_path := self.properties.get(WAREHOUSE_LOCATION):
+ warehouse_path = warehouse_path.rstrip("/")
+ return f"{warehouse_path}/{database_name}/{table_name}"
+
+ raise ValueError("No default path is set, please specify a location when creating a table")
+
+ def _get_hive_style_warehouse_location(self, database_name: str, table_name: str) -> str:
+ """Return the default warehouse location following the Hive convention of `warehousePath/databaseName.db/tableName`."""
database_properties = self.load_namespace_properties(database_name)
if database_location := database_properties.get(LOCATION):
database_location = database_location.rstrip("/")
diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py
index 29df2582..3b377626 100644
--- a/pyiceberg/catalog/dynamodb.py
+++ b/pyiceberg/catalog/dynamodb.py
@@ -664,6 +664,10 @@ class DynamoDbCatalog(MetastoreCatalog):
catalog=self,
)
+ def _get_default_warehouse_location(self, database_name: str, table_name: str) -> str:
+ """Override the default warehouse location to follow Hive-style conventions."""
+ return self._get_hive_style_warehouse_location(database_name, table_name)
+
def _get_create_table_item(database_name: str, table_name: str, properties: Properties, metadata_location: str) -> Dict[str, Any]:
current_timestamp_ms = str(round(time() * 1000))
diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py
index 25e7d3f7..88ad8aa4 100644
--- a/pyiceberg/catalog/glue.py
+++ b/pyiceberg/catalog/glue.py
@@ -821,3 +821,7 @@ class GlueCatalog(MetastoreCatalog):
@staticmethod
def __is_iceberg_table(table: "TableTypeDef") -> bool:
return table.get("Parameters", {}).get(TABLE_TYPE, "").lower() == ICEBERG
+
+ def _get_default_warehouse_location(self, database_name: str, table_name: str) -> str:
+ """Override the default warehouse location to follow Hive-style conventions."""
+ return self._get_hive_style_warehouse_location(database_name, table_name)
diff --git a/pyiceberg/catalog/hive.py b/pyiceberg/catalog/hive.py
index 0fd6b9d7..54e5100e 100644
--- a/pyiceberg/catalog/hive.py
+++ b/pyiceberg/catalog/hive.py
@@ -798,3 +798,7 @@ class HiveCatalog(MetastoreCatalog):
def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError
+
+ def _get_default_warehouse_location(self, database_name: str, table_name: str) -> str:
+ """Override the default warehouse location to follow Hive-style conventions."""
+ return self._get_hive_style_warehouse_location(database_name, table_name)
diff --git a/tests/catalog/test_sql.py b/tests/catalog/test_sql.py
index 8c3047b2..23595148 100644
--- a/tests/catalog/test_sql.py
+++ b/tests/catalog/test_sql.py
@@ -72,7 +72,7 @@ def catalog_name() -> str:
@pytest.fixture(name="random_table_identifier")
def fixture_random_table_identifier(warehouse: Path, database_name: str, table_name: str) -> Identifier:
- os.makedirs(f"{warehouse}/{database_name}.db/{table_name}/metadata/", exist_ok=True)
+ os.makedirs(f"{warehouse}/{database_name}/{table_name}/metadata/", exist_ok=True)
return database_name, table_name
@@ -80,13 +80,13 @@ def fixture_random_table_identifier(warehouse: Path, database_name: str, table_n
def fixture_another_random_table_identifier(warehouse: Path, database_name: str, table_name: str) -> Identifier:
database_name = database_name + "_new"
table_name = table_name + "_new"
- os.makedirs(f"{warehouse}/{database_name}.db/{table_name}/metadata/", exist_ok=True)
+ os.makedirs(f"{warehouse}/{database_name}/{table_name}/metadata/", exist_ok=True)
return database_name, table_name
@pytest.fixture(name="random_hierarchical_identifier")
def fixture_random_hierarchical_identifier(warehouse: Path, hierarchical_namespace_name: str, table_name: str) -> Identifier:
- os.makedirs(f"{warehouse}/{hierarchical_namespace_name}.db/{table_name}/metadata/", exist_ok=True)
+ os.makedirs(f"{warehouse}/{hierarchical_namespace_name}/{table_name}/metadata/", exist_ok=True)
return Catalog.identifier_to_tuple(".".join((hierarchical_namespace_name, table_name)))
@@ -96,7 +96,7 @@ def fixture_another_random_hierarchical_identifier(
) -> Identifier:
hierarchical_namespace_name = hierarchical_namespace_name + "_new"
table_name = table_name + "_new"
- os.makedirs(f"{warehouse}/{hierarchical_namespace_name}.db/{table_name}/metadata/", exist_ok=True)
+ os.makedirs(f"{warehouse}/{hierarchical_namespace_name}/{table_name}/metadata/", exist_ok=True)
return Catalog.identifier_to_tuple(".".join((hierarchical_namespace_name, table_name)))
@@ -115,7 +115,7 @@ def catalog_memory(catalog_name: str, warehouse: Path) -> Generator[SqlCatalog,
@pytest.fixture(scope="module")
def catalog_sqlite(catalog_name: str, warehouse: Path) -> Generator[SqlCatalog, None, None]:
props = {
- "uri": f"sqlite:////{warehouse}/sql-catalog.db",
+ "uri": f"sqlite:////{warehouse}/sql-catalog",
"warehouse": f"file://{warehouse}",
}
catalog = SqlCatalog(catalog_name, **props)
@@ -126,7 +126,7 @@ def catalog_sqlite(catalog_name: str, warehouse: Path) -> Generator[SqlCatalog,
@pytest.fixture(scope="module")
def catalog_uri(warehouse: Path) -> str:
- return f"sqlite:////{warehouse}/sql-catalog.db"
+ return f"sqlite:////{warehouse}/sql-catalog"
@pytest.fixture(scope="module")
@@ -137,7 +137,7 @@ def alchemy_engine(catalog_uri: str) -> Engine:
@pytest.fixture(scope="module")
def catalog_sqlite_without_rowcount(catalog_name: str, warehouse: Path) -> Generator[SqlCatalog, None, None]:
props = {
- "uri": f"sqlite:////{warehouse}/sql-catalog.db",
+ "uri": f"sqlite:////{warehouse}/sql-catalog",
"warehouse": f"file://{warehouse}",
}
catalog = SqlCatalog(catalog_name, **props)
@@ -150,7 +150,7 @@ def catalog_sqlite_without_rowcount(catalog_name: str, warehouse: Path) -> Gener
@pytest.fixture(scope="module")
def catalog_sqlite_fsspec(catalog_name: str, warehouse: Path) -> Generator[SqlCatalog, None, None]:
props = {
- "uri": f"sqlite:////{warehouse}/sql-catalog.db",
+ "uri": f"sqlite:////{warehouse}/sql-catalog",
"warehouse": f"file://{warehouse}",
PY_IO_IMPL: FSSPEC_FILE_IO,
}
@@ -176,7 +176,7 @@ def test_creation_with_echo_parameter(catalog_name: str, warehouse: Path) -> Non
for echo_param, expected_echo_value in test_cases:
props = {
- "uri": f"sqlite:////{warehouse}/sql-catalog.db",
+ "uri": f"sqlite:////{warehouse}/sql-catalog",
"warehouse": f"file://{warehouse}",
}
# None is for default value
@@ -199,7 +199,7 @@ def test_creation_with_pool_pre_ping_parameter(catalog_name: str, warehouse: Pat
for pool_pre_ping_param, expected_pool_pre_ping_value in test_cases:
props = {
- "uri": f"sqlite:////{warehouse}/sql-catalog.db",
+ "uri": f"sqlite:////{warehouse}/sql-catalog",
"warehouse": f"file://{warehouse}",
}
# None is for default value
@@ -219,7 +219,7 @@ def test_creation_from_impl(catalog_name: str, warehouse: Path) -> None:
catalog_name,
**{
"py-catalog-impl": "pyiceberg.catalog.sql.SqlCatalog",
- "uri": f"sqlite:////{warehouse}/sql-catalog.db",
+ "uri": f"sqlite:////{warehouse}/sql-catalog",
"warehouse": f"file://{warehouse}",
},
),
@@ -493,7 +493,7 @@ def test_create_table_with_given_location_removes_trailing_slash(
identifier_tuple = Catalog.identifier_to_tuple(table_identifier)
namespace = Catalog.namespace_from(table_identifier)
table_name = Catalog.table_name_from(identifier_tuple)
- location = f"file://{warehouse}/{catalog.name}.db/{table_name}-given"
+ location = f"file://{warehouse}/{catalog.name}/{table_name}-given"
catalog.create_namespace(namespace)
catalog.create_table(table_identifier, table_schema_nested, location=f"{location}/")
table = catalog.load_table(table_identifier)
@@ -1235,7 +1235,7 @@ def test_load_namespace_properties(catalog: SqlCatalog, namespace: str) -> None:
warehouse_location = "/test/location"
test_properties = {
"comment": "this is a test description",
- "location": f"{warehouse_location}/{namespace}.db",
+ "location": f"{warehouse_location}/{namespace}",
"test_property1": "1",
"test_property2": "2",
"test_property3": "3",
@@ -1286,7 +1286,7 @@ def test_update_namespace_properties(catalog: SqlCatalog, namespace: str) -> Non
warehouse_location = "/test/location"
test_properties = {
"comment": "this is a test description",
- "location": f"{warehouse_location}/{namespace}.db",
+ "location": f"{warehouse_location}/{namespace}",
"test_property1": "1",
"test_property2": "2",
"test_property3": "3",
@@ -1306,7 +1306,7 @@ def test_update_namespace_properties(catalog: SqlCatalog, namespace: str) -> Non
"comment": "updated test description",
"test_property4": "4",
"test_property5": "5",
- "location": f"{warehouse_location}/{namespace}.db",
+ "location": f"{warehouse_location}/{namespace}",
}
diff --git a/tests/cli/test_console.py b/tests/cli/test_console.py
index 70e04071..fe375eb2 100644
--- a/tests/cli/test_console.py
+++ b/tests/cli/test_console.py
@@ -271,7 +271,7 @@ def test_location(catalog: InMemoryCatalog) -> None:
runner = CliRunner()
result = runner.invoke(run, ["location", "default.my_table"])
assert result.exit_code == 0
- assert result.output == f"""{catalog._warehouse_location}/default.db/my_table\n"""
+ assert result.output == f"""{catalog._warehouse_location}/default/my_table\n"""
def test_location_does_not_exists(catalog: InMemoryCatalog) -> None:
@@ -700,7 +700,7 @@ def test_json_location(catalog: InMemoryCatalog) -> None:
runner = CliRunner()
result = runner.invoke(run, ["--output=json", "location", "default.my_table"])
assert result.exit_code == 0
- assert result.output == f'"{catalog._warehouse_location}/default.db/my_table"\n'
+ assert result.output == f'"{catalog._warehouse_location}/default/my_table"\n'
def test_json_location_does_not_exists(catalog: InMemoryCatalog) -> None: