You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "GayathriSrividya (via GitHub)" <gi...@apache.org> on 2026/06/18 12:13:17 UTC

[PR] fix: correct NOT STARTS WITH projection for truncated partitions [iceberg-python]

GayathriSrividya opened a new pull request, #3528:
URL: https://github.com/apache/iceberg-python/pull/3528

   closes #3493
   
   ## Summary
   
   Fixes incorrect projection of `NOT STARTS WITH` predicates for truncated string/binary partition fields. The current implementation unsafely truncates the filter literal without checking its length relative to the truncate width.
   
   ## Root Cause
   
   The `TruncateTransform.project` method calls `_truncate_array` which blindly truncates the literal for both `STARTS WITH` and `NOT STARTS WITH` predicates:
   
   ```python
   elif isinstance(pred, BoundNotStartsWith):
       return NotStartsWith(Reference(name), _transform_literal(transform, boundary))
   ```
   
   For `NOT STARTS WITH "hello"` with `truncate[2]`, this produces:
   - Current (unsafe): `NOT STARTS WITH "he"` 
   - Problem: The truncated partition contains all values starting with "he" (from "hello", "heat", "hear", etc.), so we cannot safely exclude all non-"hello" rows
   
   ## Solution
   
   Add special handling for `BoundNotStartsWith` in the `project` method following the Java/Go reference behavior:
   
   - **prefix_length < truncate_width**: Keep original `NOT STARTS WITH` literal (safe)
   - **prefix_length == truncate_width**: Project to `!=` instead (safe equality check)
   - **prefix_length > truncate_width**: Return `None` (no inclusive projection possible)
   
   ### pyiceberg/transforms.py
   - Add explicit `NOT STARTS WITH` handling before calling `_truncate_array`
   - Check literal length vs truncate width and apply correct projection rules
   
   ### tests/test_transforms.py
   - Update `test_projection_truncate_string_not_starts_with` to expect `None` (prefix_length > width is unsafe)
   - Add `test_projection_truncate_string_not_starts_with_shorter_literal` (prefix_length == width → `!=`)
   - Add `test_projection_truncate_string_not_starts_with_original_literal` (prefix_length < width → original)
   
   ## Validation
   - `make lint` ✓ (all pre-commit hooks pass)
   - `pytest tests/test_transforms.py` → 280 passed ✓
   - All 13 string truncate projection tests pass


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] fix: correct NOT STARTS WITH projection for truncated partitions [iceberg-python]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #3528:
URL: https://github.com/apache/iceberg-python/pull/3528#issuecomment-4763089433

   Thanks for the contribution @GayathriSrividya
   
   Just FYI there was already another PR opened before this one, #3519. Would be great to coordinate on the original issue next time and not duplicate work. I merged this one because it has a more complete solution than the other PR. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] fix: correct NOT STARTS WITH projection for truncated partitions [iceberg-python]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #3528:
URL: https://github.com/apache/iceberg-python/pull/3528


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] fix: correct NOT STARTS WITH projection for truncated partitions [iceberg-python]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #3528:
URL: https://github.com/apache/iceberg-python/pull/3528#discussion_r3448915556


##########
tests/test_transforms.py:
##########
@@ -1026,9 +1026,22 @@ def test_projection_truncate_string_starts_with(bound_reference_str: BoundRefere
 
 
 def test_projection_truncate_string_not_starts_with(bound_reference_str: BoundReference) -> None:
+    # literal_width (5) > truncate width (2): no inclusive projection possible (unsafe)

Review Comment:
   ```suggestion
   def test_projection_truncate_string_not_starts_with_longer_literal(bound_reference_str: BoundReference) -> None:
       # Not a valid projection: return None because pruning on "he" could drop qualifying rows like "help".
   ```



##########
tests/test_transforms.py:
##########
@@ -1026,9 +1026,22 @@ def test_projection_truncate_string_starts_with(bound_reference_str: BoundRefere
 
 
 def test_projection_truncate_string_not_starts_with(bound_reference_str: BoundReference) -> None:
+    # literal_width (5) > truncate width (2): no inclusive projection possible (unsafe)
+    assert TruncateTransform(2).project("name", BoundNotStartsWith(term=bound_reference_str, literal=literal("hello"))) is None
+
+
+def test_projection_truncate_string_not_starts_with_shorter_literal(bound_reference_str: BoundReference) -> None:
+    # literal_width (2) == truncate width (2): project to !=

Review Comment:
   ```suggestion
   def test_projection_truncate_string_not_starts_with_equal_width_literal(bound_reference_str: BoundReference) -> None:
       # Valid projection: improve NOT STARTS WITH "he" to partition != "he".
   ```



##########
tests/test_transforms.py:
##########
@@ -1026,9 +1026,22 @@ def test_projection_truncate_string_starts_with(bound_reference_str: BoundRefere
 
 
 def test_projection_truncate_string_not_starts_with(bound_reference_str: BoundReference) -> None:
+    # literal_width (5) > truncate width (2): no inclusive projection possible (unsafe)
+    assert TruncateTransform(2).project("name", BoundNotStartsWith(term=bound_reference_str, literal=literal("hello"))) is None
+
+
+def test_projection_truncate_string_not_starts_with_shorter_literal(bound_reference_str: BoundReference) -> None:
+    # literal_width (2) == truncate width (2): project to !=
+    assert TruncateTransform(2).project(
+        "name", BoundNotStartsWith(term=bound_reference_str, literal=literal("he"))
+    ) == NotEqualTo(term="name", literal=literal("he"))
+
+
+def test_projection_truncate_string_not_starts_with_original_literal(bound_reference_str: BoundReference) -> None:
+    # literal_width (1) < truncate width (2): keep original literal

Review Comment:
   ```suggestion
   def test_projection_truncate_string_not_starts_with_shorter_literal(bound_reference_str: BoundReference) -> None:
       # Valid projection: pass the NOT STARTS WITH literal "h" through unchanged.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org