amogh-jahagirdar commented on code in PR #246:
URL: https://github.com/apache/iceberg-python/pull/246#discussion_r1439112448
##########
pyiceberg/expressions/visitors.py:
##########
@@ -906,7 +906,16 @@ def visit_bound_predicate(self, predicate:
BoundPredicate[L]) -> BooleanExpressi
def translate_column_names(expr: BooleanExpression, file_schema: Schema,
case_sensitive: bool) -> BooleanExpression:
- return visit(expr, _ColumnNameTranslator(file_schema, case_sensitive))
+ try:
+ return visit(expr, _ColumnNameTranslator(file_schema, case_sensitive))
+ except ValueError as e:
+ if "Not found in file schema" in str(e):
+ if isinstance(expr, BoundIsNull):
+ return AlwaysTrue()
+ else:
+ return AlwaysFalse()
Review Comment:
Curious, why not do this in the visitor itself
(_ColumnNameTranslator#visit_bound_predicate)? Is it something we would want
decoupled from it?
##########
tests/test_integration.py:
##########
@@ -373,6 +379,15 @@ def test_scan_branch(test_positional_mor_deletes: Table)
-> None:
assert arrow_table["number"].to_pylist() == [1, 2, 3, 4, 6, 7, 8, 9, 10,
11, 12]
+@pytest.mark.integration
+def test_filter_on_new_column(test_table_add_column: Table) -> None:
+ arrow_table = test_table_add_column.scan(row_filter="b == '2'").to_arrow()
+ assert arrow_table["b"].to_pylist() == ['2']
Review Comment:
As a sanity check should we add a test which validates that a row_filter='b
is NOT NULL' returns the same?
--
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
