Fokko commented on code in PR #1693:
URL: https://github.com/apache/iceberg-python/pull/1693#discussion_r1965153625


##########
tests/table/test_upsert.py:
##########
@@ -366,3 +367,22 @@ def test_upsert_with_identifier_fields(catalog: Catalog) 
-> None:
 
     assert upd.rows_updated == 1
     assert upd.rows_inserted == 1
+
+
+def test_create_match_filter_single_condition() -> None:
+    """
+    Test create_match_filter with a composite key where the source yields 
exactly one unique key.
+    Expected: The function returns the single And condition directly.
+    """
+
+    data = [
+        {"order_id": 101, "order_line_id": 1, "extra": "x"},
+        {"order_id": 101, "order_line_id": 1, "extra": "x"},  # duplicate
+    ]
+    schema = pa.schema([pa.field("order_id", pa.int32()), 
pa.field("order_line_id", pa.int32()), pa.field("extra", pa.string())])
+    table = pa.Table.from_pylist(data, schema=schema)
+    expr = create_match_filter(table, ["order_id", "order_line_id"])
+    expr_str = str(expr)
+
+    assert "And(" in expr_str, f"Expected And condition but got: {expr_str}"
+    assert "Or(" not in expr_str, "Did not expect an Or condition when only 
one unique key exists"

Review Comment:
   @omkenge I think converting it into a string is not the best approach as it 
doesn't cover the full result. I'd rather see something like:
   
   ```suggestion
       assert expr == And(
           EqualTo(term=Reference(name="order_id"), literal=LongLiteral(101)),
           right=EqualTo(term=Reference(name="order_line_id"), 
literal=LongLiteral(1)),
       )
   ```
   
   In my opinion, when we fix a bug, we always should add a test-case. 
Otherwise, we might break it again later.



-- 
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-unsubscr...@iceberg.apache.org

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


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

Reply via email to