JingsongLi commented on code in PR #10175:
URL: https://github.com/apache/paimon/pull/10175#discussion_r4179947709
##########
paimon-python/pypaimon/common/predicate_builder.py:
##########
@@ -34,6 +38,49 @@ def _get_field_index(self, field: str) -> int:
except ValueError:
raise ValueError(f'The field {field} is not in field list
{self.field_names}.')
+ def _validate_array_field(self, method: str, field: str) -> None:
+ """Reject a non-ARRAY field, mirroring Java
``ArrayContains.elementType``
+ which requires an ARRAY column. Without this, the row-level testers run
+ ``literal in value`` and would match against a STRING (or other)
column,
+ silently selecting wrong rows on a wrong name or after a schema change.
+ """
+ if field not in self._field_types:
+ raise ValueError(f'The field {field} is not in field list
{self.field_names}.')
+ field_type = self._field_types[field]
+ if not isinstance(field_type, ArrayType):
+ raise ValueError(
+ "{} requires an ARRAY field, but '{}' is {}.".format(method,
field, field_type))
+
+ def _normalize_array_literals(self, field: str, literals: List[Any]) ->
List[Any]:
+ """Normalize element literals to the array's declared element type.
+
+ A FLOAT array stores float32 values, so a Python ``float`` literal
+ (double) such as ``0.1`` must be rounded to float32 before comparison;
+ otherwise ``array_contains`` never matches the stored ``0.1``. A DOUBLE
+ array needs an integer literal (e.g. ``0``) widened to ``float`` so it
+ reaches the signed-zero-aware comparator rather than Python ``==``
+ (otherwise ``0`` and ``0.0`` select different rows). int/str/bool
+ element types compare exactly and need no normalization.
+ """
+ element = self._field_types[field].element
+ if isinstance(element, AtomicType) and element.type == 'FLOAT':
+ return [self._to_float32(literal) for literal in literals]
+ if isinstance(element, AtomicType) and element.type == 'DOUBLE':
+ return [self._to_double(literal) for literal in literals]
+ return literals
+
+ @staticmethod
+ def _to_double(value: Any) -> Any:
+ if not isinstance(value, (int, float)) or isinstance(value, bool):
+ return value
+ return float(value)
+
+ @staticmethod
+ def _to_float32(value: Any) -> Any:
+ if not isinstance(value, (int, float)) or isinstance(value, bool):
+ return value
+ return struct.unpack('<f', struct.pack('<f', float(value)))[0]
Review Comment:
[P2] Handle finite FLOAT literal overflow during normalization
This new conversion accepts Python int/float literals but `struct.pack("<f",
...)` raises `OverflowError` for a finite double such as `1e40`. On actual
committed and reopened ARRAY<FLOAT> tables, `array_contains("floats", 1e40)`,
`arrays_overlap("floats", [1e40])` and `array_contains_all("floats", [1e40])`
all fail before reading, in Parquet, Avro, ORC and row formats. The same APIs
with `float("inf")` correctly match the stored positive-infinity row. This is
inconsistent with the typed normalization used for 0.1: Java
`PredicateBuilder.convertJavaObject(FLOAT, Double.valueOf(1e40))` uses
`Number.floatValue()` and yields positive infinity, as does PyArrow float32
conversion. A source-free control changing only overflow conversion to the
corresponding signed infinity makes the three queries return the correct row
IDs, including ID-only projection and compound OR. Please preserve that float32
conversion behavior for finite overflow (both signs) and add a regression test
for the thre
e public methods. This concerns finite float overflow, independent of the
excluded signed-zero comparison issue.
--
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]
