Copilot commented on code in PR #4278:
URL: https://github.com/apache/iceberg-python/pull/4278#discussion_r4459956298


##########
pyiceberg/table/__init__.py:
##########
@@ -2557,12 +2578,33 @@ def count(self) -> int:
         res = 0
         # every task is a FileScanTask
         tasks = self.plan_files()
+        bound_filter = bind(self.table_metadata.schema(), self.row_filter, 
case_sensitive=self.case_sensitive)
+        field_ids_outside_null_checks = visit(bound_filter, 
_FieldIDsOutsideNullChecks())
+        referenced_positions = (
+            {
+                spec_id: [pos for pos, field in enumerate(spec.fields) if 
field.source_id in field_ids_outside_null_checks]
+                for spec_id, spec in self.table_metadata.specs().items()
+            }
+            if field_ids_outside_null_checks
+            else {}
+        )
 
         for task in tasks:
             # task.residual is a Boolean Expression if the filter condition is 
fully satisfied by the
             # partition value and task.delete_files represents that positional 
delete haven't been merged yet
             # hence those files have to read as a pyarrow table applying the 
filter and deletes
-            if task.residual == AlwaysTrue() and len(task.delete_files) == 0:
+            if (
+                task.residual == AlwaysTrue()
+                and len(task.delete_files) == 0
+                # A NULL partition value can leave an AlwaysTrue residual 
although the filter rejects its rows
+                and (
+                    not field_ids_outside_null_checks
+                    or (
+                        task.file.spec_id in referenced_positions
+                        and all(task.file.partition[pos] is not None for pos 
in referenced_positions[task.file.spec_id])

Review Comment:
   `RESTContentFile.partition` defaults to an empty list, which 
`_rest_file_to_data_file()` converts to `Record()`. For a server-planned task 
with an `AlwaysTrue` residual, a known partitioned spec, and a filter 
referencing its source column, this indexing raises `IndexError` when the 
response omits partition values. Treat missing positions like an unknown spec 
and fall back to reading the file; add a regression case for an omitted REST 
partition.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to