(iceberg-python) branch gh-readonly-queue/main/pr-4005-4dba4a480bfe7548bcb99e1c4194f692d2975d00 created (now d888dd113)

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-4005-4dba4a480bfe7548bcb99e1c4194f692d2975d00
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git


      at d888dd113 fix(config): let an explicitly falsy value on the right-hand side win in merge_config (#4005)

This branch includes the following new commits:

     new d888dd113 fix(config): let an explicitly falsy value on the right-hand side win in merge_config (#4005)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(iceberg-python) 01/01: fix(config): let an explicitly falsy value on the right-hand side win in merge_config (#4005)

[gi...@apache.org]

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-4005-4dba4a480bfe7548bcb99e1c4194f692d2975d00
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git

commit d888dd113efc5bddd650ccdd17c64f8866ccf07b
Author: Rodrigo Stachlewski Palma <86...@users.noreply.github.com>
AuthorDate: Thu Sep 24 19:44:47 2026 +0000

    fix(config): let an explicitly falsy value on the right-hand side win in merge_config (#4005)
    
    fix(config): let an explicitly falsy value on the right-hand side win
    
    merge_config() documents 'take the non-null value, with precedence on
    rhs', but it used `rhs_value or lhs_value`, which falls back to the
    left-hand side for any falsy value, not just None.
    
    load_catalog(name, **properties) merges the configuration file into the
    caller's properties, so turning an option off explicitly was silently
    ignored when the file set it:
    
        # .pyiceberg.yaml has s3.path-style-access: "true"
        load_catalog("prod", **{"s3.path-style-access": False})
        # -> 's3.path-style-access' stays 'true'
    
    That contradicts the intent of #45, which made the passed-in argument
    take precedence over the environment. property_as_bool() in the same
    codebase already distinguishes 'not set' from falsy by testing against
    (None, ""), so the two now agree.
    
    None on the right-hand side still means 'not set' and keeps the
    left-hand value.
---
 pyiceberg/utils/config.py  |  5 +++--
 tests/utils/test_config.py | 24 +++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/pyiceberg/utils/config.py b/pyiceberg/utils/config.py
index 2b5baafa5..4d58365d5 100644
--- a/pyiceberg/utils/config.py
+++ b/pyiceberg/utils/config.py
@@ -42,8 +42,9 @@ def merge_config(lhs: RecursiveDict, rhs: RecursiveDict) -> RecursiveDict:
                 # If they are both dicts, then we have to go deeper
                 new_config[rhs_key] = merge_config(lhs_value, rhs_value)
             else:
-                # Take the non-null value, with precedence on rhs
-                new_config[rhs_key] = rhs_value or lhs_value
+                # Take the non-null value, with precedence on rhs. `None` means "not set",
+                # while a falsy value such as `False` or `0` is an explicit setting and wins.
+                new_config[rhs_key] = rhs_value if rhs_value is not None else lhs_value
         else:
             # New key
             new_config[rhs_key] = rhs_value
diff --git a/tests/utils/test_config.py b/tests/utils/test_config.py
index 309821023..17b495f92 100644
--- a/tests/utils/test_config.py
+++ b/tests/utils/test_config.py
@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 import os
-from typing import Any
+from typing import Any, cast
 from unittest import mock
 
 import pytest
@@ -87,6 +87,28 @@ def test_merge_config() -> None:
     assert result["common_key"] == rhs["common_key"]
 
 
+@pytest.mark.parametrize("falsy_value", [False, "", 0])
+def test_merge_config_rhs_wins_for_falsy_values(falsy_value: Any) -> None:
+    """A value set explicitly on the right-hand side wins even when it is falsy.
+
+    `load_catalog(name, **properties)` merges the configuration file into the properties
+    passed by the caller, so turning an option off explicitly must not fall back to the
+    value coming from the file.
+    """
+    lhs: RecursiveDict = {"s3.path-style-access": "true"}
+    rhs: RecursiveDict = {"s3.path-style-access": falsy_value}
+    result = merge_config(lhs, rhs)
+    assert result["s3.path-style-access"] == falsy_value
+
+
+def test_merge_config_lhs_wins_when_rhs_is_none() -> None:
+    """`None` on the right-hand side means "not set", so the left-hand side survives."""
+    lhs: RecursiveDict = {"uri": "https://example.com"}
+    rhs = cast(RecursiveDict, {"uri": None})
+    result = merge_config(lhs, rhs)
+    assert result["uri"] == "https://example.com"
+
+
 def test_from_configuration_files_get_typed_value(tmp_path_factory: pytest.TempPathFactory) -> None:
     config_path = str(tmp_path_factory.mktemp("config"))
     with open(f"{config_path}/.pyiceberg.yaml", "w", encoding=UTF8) as file: