zeroshade commented on code in PR #891:
URL: https://github.com/apache/iceberg-go/pull/891#discussion_r3087712706


##########
table/evaluators.go:
##########
@@ -1563,3 +1564,187 @@ func (m *strictMetricsEval) canContainNans(fieldID int) 
bool {
 
        return exists && cnt > 0
 }
+
+// literalToPhysBytes converts an Iceberg literal to the physical byte
+// representation that the Parquet bloom filter writer hashes, using the
+// literal's MarshalBinary encoding. Float types are normalized before
+// encoding (-0.0 → 0.0, NaN → canonical NaN) to match Parquet/Java writer
+// behavior and avoid false-negative bloom filter results.
+func literalToPhysBytes(lit iceberg.Literal) ([]byte, bool) {
+       switch v := lit.(type) {
+       case iceberg.Float32Literal:
+               // Normalize: -0.0 and 0.0 have different bit patterns, so 
without this
+               // a query for 0.0 won't match a row group where -0.0 was 
written.
+               f := float32(v)
+               if f == 0 {
+                       f = 0
+               } else if math.IsNaN(float64(f)) {
+                       f = float32(math.NaN())
+               }
+
+               lit = iceberg.Float32Literal(f)
+
+       case iceberg.Float64Literal:
+               f := float64(v)
+               if f == 0 {
+                       f = 0
+               } else if math.IsNaN(f) {
+                       f = math.NaN()
+               }
+
+               lit = iceberg.Float64Literal(f)
+
+       case iceberg.BoolLiteral:
+               // Low cardinality; bloom filters not useful.
+               return nil, false
+
+       case iceberg.DecimalLiteral:
+               // Physical representation depends on precision 
(INT32/INT64/FIXED_LEN_BYTE_ARRAY).
+               // TODO: add decimal bloom filter support once schema-level 
precision lookup is wired in.
+               _ = v
+
+               return nil, false
+
+       case iceberg.TimestampNsLiteral:
+               // Not yet handled by all expression machinery; skip for safety.
+               _ = v
+
+               return nil, false
+       }
+
+       type binaryMarshaler interface {
+               MarshalBinary() ([]byte, error)
+       }

Review Comment:
   this already exists as `encoding.BinaryMarshaler` you don't need to create a 
new interface for this



-- 
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