You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by gi...@apache.org on 2026/09/24 19:14:50 UTC
(iceberg-python) 01/01: fix: rank catalog configuration above table properties when constructing FileIO (#3956)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3956-0d584073f67948c46f34d4c2a1672455bc10b5c9
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
commit a46cbdf09a77bf893d05c3e49caf6b72b3c24d57
Author: Yuang Gao <91...@users.noreply.github.com>
AuthorDate: Thu Sep 24 19:14:24 2026 +0000
fix: rank catalog configuration above table properties when constructing FileIO (#3956)
* Rank catalog configuration above table properties when constructing FileIO
* Cover FileIO property precedence in StaticTable.from_metadata
---
pyiceberg/catalog/__init__.py | 15 +++++++++++----
pyiceberg/catalog/bigquery_metastore.py | 2 +-
pyiceberg/catalog/dynamodb.py | 2 +-
pyiceberg/catalog/glue.py | 6 +++---
pyiceberg/catalog/hive.py | 2 +-
pyiceberg/catalog/rest/__init__.py | 15 +++++++++++----
pyiceberg/catalog/sql.py | 2 +-
pyiceberg/table/__init__.py | 2 +-
tests/catalog/test_rest.py | 28 +++++++++++++++++++++++++++-
tests/catalog/test_sql.py | 20 ++++++++++++++++++++
tests/table/test_init.py | 6 ++++++
11 files changed, 83 insertions(+), 17 deletions(-)
diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py
index 8de113404..0842f51cc 100644
--- a/pyiceberg/catalog/__init__.py
+++ b/pyiceberg/catalog/__init__.py
@@ -833,8 +833,15 @@ class Catalog(ABC):
return tuple_identifier[0], tuple_identifier[1]
- def _load_file_io(self, properties: Properties = EMPTY_DICT, location: str | None = None) -> FileIO:
- return load_file_io({**self.properties, **properties}, location)
+ def _load_file_io(
+ self,
+ properties: Properties = EMPTY_DICT,
+ location: str | None = None,
+ table_properties: Properties = EMPTY_DICT,
+ ) -> FileIO:
+ # table_properties ranks lowest: a principal who can commit to a table must not
+ # be able to redirect its readers.
+ return load_file_io({**table_properties, **self.properties, **properties}, location)
@staticmethod
def _convert_schema_if_needed(
@@ -1020,7 +1027,7 @@ class MetastoreCatalog(Catalog, ABC):
metadata = new_table_metadata(
location=location, schema=schema, partition_spec=partition_spec, sort_order=sort_order, properties=properties
)
- io = self._load_file_io(properties=properties, location=metadata_location)
+ io = self._load_file_io(location=metadata_location, table_properties=properties)
return StagedTable(
identifier=(database_name, table_name),
metadata=metadata,
@@ -1054,7 +1061,7 @@ class MetastoreCatalog(Catalog, ABC):
identifier=table_identifier,
metadata=updated_metadata,
metadata_location=new_metadata_location,
- io=self._load_file_io(properties=updated_metadata.properties, location=new_metadata_location),
+ io=self._load_file_io(location=new_metadata_location, table_properties=updated_metadata.properties),
catalog=self,
)
diff --git a/pyiceberg/catalog/bigquery_metastore.py b/pyiceberg/catalog/bigquery_metastore.py
index cc84b0b42..1267233e9 100644
--- a/pyiceberg/catalog/bigquery_metastore.py
+++ b/pyiceberg/catalog/bigquery_metastore.py
@@ -404,7 +404,7 @@ class BigQueryMetastoreCatalog(MetastoreCatalog):
identifier=(dataset_name, table_name),
metadata=metadata,
metadata_location=metadata_location,
- io=self._load_file_io(metadata.properties, metadata_location),
+ io=self._load_file_io(location=metadata_location, table_properties=metadata.properties),
catalog=self,
)
diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py
index 10f74fb9a..b4111a6fb 100644
--- a/pyiceberg/catalog/dynamodb.py
+++ b/pyiceberg/catalog/dynamodb.py
@@ -697,7 +697,7 @@ class DynamoDbCatalog(MetastoreCatalog):
identifier=(database_name, table_name),
metadata=metadata,
metadata_location=metadata_location,
- io=self._load_file_io(metadata.properties, metadata_location),
+ io=self._load_file_io(location=metadata_location, table_properties=metadata.properties),
catalog=self,
)
diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py
index 12b36efc5..977876918 100644
--- a/pyiceberg/catalog/glue.py
+++ b/pyiceberg/catalog/glue.py
@@ -389,7 +389,7 @@ class GlueCatalog(MetastoreCatalog):
identifier=(database_name, table_name),
metadata=metadata,
metadata_location=metadata_location,
- io=self._load_file_io(metadata.properties, metadata_location),
+ io=self._load_file_io(location=metadata_location, table_properties=metadata.properties),
catalog=self,
)
@@ -535,7 +535,7 @@ class GlueCatalog(MetastoreCatalog):
identifier=self.identifier_to_tuple(identifier),
metadata=staged_table.metadata,
metadata_location=staged_table.metadata_location,
- io=self._load_file_io(staged_table.metadata.properties, staged_table.metadata_location),
+ io=self._load_file_io(location=staged_table.metadata_location, table_properties=staged_table.metadata.properties),
catalog=self,
)
@@ -599,7 +599,7 @@ class GlueCatalog(MetastoreCatalog):
identifier=self.identifier_to_tuple(identifier),
metadata=staged_table.metadata,
metadata_location=staged_table.metadata_location,
- io=self._load_file_io(staged_table.metadata.properties, staged_table.metadata_location),
+ io=self._load_file_io(location=staged_table.metadata_location, table_properties=staged_table.metadata.properties),
catalog=self,
)
diff --git a/pyiceberg/catalog/hive.py b/pyiceberg/catalog/hive.py
index ed6d3c6fd..a44da81b6 100644
--- a/pyiceberg/catalog/hive.py
+++ b/pyiceberg/catalog/hive.py
@@ -353,7 +353,7 @@ class HiveCatalog(MetastoreCatalog):
identifier=(table.dbName, table.tableName),
metadata=metadata,
metadata_location=metadata_location,
- io=self._load_file_io(metadata.properties, metadata_location),
+ io=self._load_file_io(location=metadata_location, table_properties=metadata.properties),
catalog=self,
)
diff --git a/pyiceberg/catalog/rest/__init__.py b/pyiceberg/catalog/rest/__init__.py
index d8f58773e..cb20d4ddc 100644
--- a/pyiceberg/catalog/rest/__init__.py
+++ b/pyiceberg/catalog/rest/__init__.py
@@ -638,8 +638,13 @@ class RestCatalog(Catalog):
return best_match.config if best_match else {}
- def _load_file_io(self, properties: Properties = EMPTY_DICT, location: str | None = None) -> FileIO:
- merged_properties = {**self.properties, **properties}
+ def _load_file_io(
+ self,
+ properties: Properties = EMPTY_DICT,
+ location: str | None = None,
+ table_properties: Properties = EMPTY_DICT,
+ ) -> FileIO:
+ merged_properties = {**table_properties, **self.properties, **properties}
if self._auth_manager:
merged_properties[AUTH_MANAGER] = self._auth_manager
return load_file_io(merged_properties, location)
@@ -1152,8 +1157,9 @@ class RestCatalog(Catalog):
metadata_location=table_response.metadata_location, # type: ignore
metadata=table_response.metadata,
io=self._load_file_io(
- {**table_response.metadata.properties, **table_response.config, **credential_config},
+ {**table_response.config, **credential_config},
table_response.metadata_location,
+ table_properties=table_response.metadata.properties,
),
catalog=self,
config=table_response.config,
@@ -1169,8 +1175,9 @@ class RestCatalog(Catalog):
metadata_location=table_response.metadata_location, # type: ignore
metadata=table_response.metadata,
io=self._load_file_io(
- {**table_response.metadata.properties, **table_response.config, **credential_config},
+ {**table_response.config, **credential_config},
table_response.metadata_location,
+ table_properties=table_response.metadata.properties,
),
catalog=self,
)
diff --git a/pyiceberg/catalog/sql.py b/pyiceberg/catalog/sql.py
index abe31194b..13e67997d 100644
--- a/pyiceberg/catalog/sql.py
+++ b/pyiceberg/catalog/sql.py
@@ -237,7 +237,7 @@ class SqlCatalog(MetastoreCatalog):
identifier=Catalog.identifier_to_tuple(table_namespace) + (table_name,),
metadata=metadata,
metadata_location=metadata_location,
- io=self._load_file_io(metadata.properties, metadata_location),
+ io=self._load_file_io(location=metadata_location, table_properties=metadata.properties),
catalog=self,
)
diff --git a/pyiceberg/table/__init__.py b/pyiceberg/table/__init__.py
index ac67e3187..0caba0e5d 100644
--- a/pyiceberg/table/__init__.py
+++ b/pyiceberg/table/__init__.py
@@ -1995,7 +1995,7 @@ class StaticTable(Table):
identifier=("static-table", metadata_location),
metadata_location=metadata_location,
metadata=metadata,
- io=load_file_io({**properties, **metadata.properties}, location=metadata_location),
+ io=load_file_io({**metadata.properties, **properties}, location=metadata_location),
catalog=NoopCatalog("static-table"),
)
diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py
index a918829c2..61b6a16e9 100644
--- a/tests/catalog/test_rest.py
+++ b/tests/catalog/test_rest.py
@@ -65,7 +65,7 @@ from pyiceberg.exceptions import (
TableAlreadyExistsError,
ViewAlreadyExistsError,
)
-from pyiceberg.io import load_file_io
+from pyiceberg.io import ARROW_FILE_IO, FSSPEC_FILE_IO, PY_IO_IMPL, load_file_io
from pyiceberg.partitioning import PartitionField, PartitionSpec
from pyiceberg.schema import Schema
from pyiceberg.table import Table
@@ -3415,6 +3415,32 @@ def test_load_table_with_storage_credentials(rest_mock: Mocker, example_table_me
assert table.io.properties["s3.session-token"] == "vended-token"
+def test_load_table_catalog_config_outranks_table_properties(
+ rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any]
+) -> None:
+ metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json"
+ rest_mock.get(
+ f"{TEST_URI}v1/namespaces/fokko/tables/table",
+ json={
+ "metadata-location": metadata_location,
+ "metadata": {
+ **example_table_metadata_with_snapshot_v1,
+ "properties": {PY_IO_IMPL: FSSPEC_FILE_IO, "s3.proxy-uri": "http://table-only-proxy"},
+ },
+ "config": {"s3.region": "from-config"},
+ },
+ status_code=200,
+ request_headers=TEST_HEADERS,
+ )
+ catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **{PY_IO_IMPL: ARROW_FILE_IO, "s3.region": "from-catalog"})
+ table = catalog.load_table(("fokko", "table"))
+
+ assert table.io.properties[PY_IO_IMPL] == ARROW_FILE_IO
+ # Server config and a key the catalog leaves unset keep working.
+ assert table.io.properties["s3.region"] == "from-config"
+ assert table.io.properties["s3.proxy-uri"] == "http://table-only-proxy"
+
+
def test_load_credentials_with_longest_prefix(rest_mock: Mocker) -> None:
rest_mock.get(
f"{TEST_URI}v1/namespaces/fokko/tables/table/credentials",
diff --git a/tests/catalog/test_sql.py b/tests/catalog/test_sql.py
index 6c1767f71..df5e30e25 100644
--- a/tests/catalog/test_sql.py
+++ b/tests/catalog/test_sql.py
@@ -36,6 +36,7 @@ from pyiceberg.exceptions import (
NoSuchTableError,
TableAlreadyExistsError,
)
+from pyiceberg.io import ARROW_FILE_IO, FSSPEC_FILE_IO, PY_IO_IMPL
from pyiceberg.schema import Schema
from pyiceberg.types import NestedField, StringType, strtobool
@@ -299,6 +300,25 @@ def test_idempotent_when_column_already_exists(warehouse: Path) -> None:
assert "iceberg_type" in get_columns(catalog.engine)
+def test_load_table_ranks_catalog_config_above_table_properties(warehouse: Path) -> None:
+ catalog = SqlCatalog(
+ name="test",
+ uri="sqlite:///:memory:",
+ warehouse=f"file://{warehouse}",
+ **{PY_IO_IMPL: ARROW_FILE_IO},
+ )
+ catalog.create_namespace("ns")
+ catalog.create_table(
+ ("ns", "tbl"),
+ Schema(NestedField(1, "id", StringType(), required=True)),
+ properties={PY_IO_IMPL: FSSPEC_FILE_IO, "s3.proxy-uri": "http://table-only-proxy"},
+ )
+
+ io = catalog.load_table(("ns", "tbl")).io
+ assert io.properties[PY_IO_IMPL] == ARROW_FILE_IO
+ assert io.properties["s3.proxy-uri"] == "http://table-only-proxy"
+
+
def test_list_tables_filters_by_iceberg_type(warehouse: Path) -> None:
catalog = SqlCatalog(
name="test",
diff --git a/tests/table/test_init.py b/tests/table/test_init.py
index 83de76059..3d160781e 100644
--- a/tests/table/test_init.py
+++ b/tests/table/test_init.py
@@ -620,6 +620,12 @@ def test_static_table_io_does_not_exist(metadata_location: str) -> None:
StaticTable.from_metadata(metadata_location, {PY_IO_IMPL: "pyiceberg.does.not.exist.FileIO"})
+def test_static_table_properties_outrank_metadata_properties(metadata_location: str) -> None:
+ static_table = StaticTable.from_metadata(metadata_location, {"read.split.target.size": "1"})
+ assert static_table.metadata.properties["read.split.target.size"] == "134217728"
+ assert static_table.io.properties["read.split.target.size"] == "1"
+
+
def test_serialize_set_properties_updates() -> None:
assert (
SetPropertiesUpdate(updates={"abc": "🤪"}).model_dump_json() == """{"action":"set-properties","updates":{"abc":"🤪"}}"""