zeroshade commented on code in PR #1478: URL: https://github.com/apache/iceberg-go/pull/1478#discussion_r3631998801
########## table/internal/variant_bounds.go: ########## @@ -0,0 +1,368 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package internal + +import ( + "fmt" + "math" + "slices" + "sort" + "strings" + "unicode/utf8" + + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/decimal" + "github.com/apache/arrow-go/v18/parquet/variant" + "github.com/apache/iceberg-go" + "github.com/google/uuid" +) + +// isNaNLiteral reports whether a float literal is NaN (bounds are skipped for it, per Java). +func isNaNLiteral(lit iceberg.Literal) bool { + switch v := lit.Any().(type) { + case float32: + return math.IsNaN(float64(v)) + case float64: + return math.IsNaN(v) + } + + return false +} + +// variantLeaf is one shredded typed_value primitive leaf that can carry a bound. +type variantLeaf struct { + jsonPath string + typedPath string + valuePaths []string + icebergType iceberg.PrimitiveType +} + +// enumerateVariantLeaves returns a shredded variant's bound-bearing primitive leaves. +func enumerateVariantLeaves(variantColPath []string, tv arrow.Field) []variantLeaf { + var out []variantLeaf + rootValue := joinPath(append(cloneStrs(variantColPath), "value")) + walkVariantTyped(append(cloneStrs(variantColPath), "typed_value"), []string{rootValue}, nil, tv.Type, &out) + + return out +} + +// walkVariantTyped collects primitive leaves from a shredded typed_value subtree, carrying every ancestor residual value path. +func walkVariantTyped(typedPath []string, valuePaths []string, fields []string, dt arrow.DataType, out *[]variantLeaf) { + switch dt.(type) { + case *arrow.StructType: + t := dt.(*arrow.StructType) + for _, f := range t.Fields() { + grp, ok := f.Type.(*arrow.StructType) + if !ok { + continue + } + tvIdx, hasTyped := grp.FieldIdx("typed_value") + if !hasTyped { + continue + } + childValues := valuePaths + if _, hasVal := grp.FieldIdx("value"); hasVal { + childValues = append(cloneStrs(valuePaths), joinPath(append(cloneStrs(typedPath), f.Name, "value"))) + } + walkVariantTyped(append(cloneStrs(typedPath), f.Name, "typed_value"), childValues, + append(cloneStrs(fields), f.Name), grp.Field(tvIdx).Type, out) + } + case *arrow.ListType, *arrow.LargeListType, *arrow.FixedSizeListType: + return + default: + it, ok := arrowLeafToIceberg(dt) + if !ok { + return + } + *out = append(*out, variantLeaf{ + jsonPath: normalizedVariantPath(fields), + typedPath: joinPath(typedPath), + valuePaths: cloneStrs(valuePaths), + icebergType: it, + }) + } +} + +// arrowLeafToIceberg maps a shredded leaf's arrow type to its Iceberg type. +func arrowLeafToIceberg(dt arrow.DataType) (iceberg.PrimitiveType, bool) { + switch t := dt.(type) { + case *arrow.BooleanType: + return iceberg.PrimitiveTypes.Bool, true + case *arrow.Int8Type, *arrow.Int16Type, *arrow.Int32Type: + return iceberg.PrimitiveTypes.Int32, true + case *arrow.Int64Type: + return iceberg.PrimitiveTypes.Int64, true + case *arrow.Float32Type: + return iceberg.PrimitiveTypes.Float32, true + case *arrow.Float64Type: + return iceberg.PrimitiveTypes.Float64, true + case *arrow.StringType: + return iceberg.PrimitiveTypes.String, true + case *arrow.BinaryType: + return iceberg.PrimitiveTypes.Binary, true + case *arrow.Date32Type: + return iceberg.PrimitiveTypes.Date, true + case *arrow.Time64Type: + if t.Unit == arrow.Microsecond { + return iceberg.PrimitiveTypes.Time, true + } + case *arrow.TimestampType: + switch t.Unit { + case arrow.Microsecond: + if t.TimeZone == "" { + return iceberg.PrimitiveTypes.Timestamp, true + } + + return iceberg.PrimitiveTypes.TimestampTz, true + case arrow.Nanosecond: + if t.TimeZone == "" { + return iceberg.PrimitiveTypes.TimestampNs, true + } + + return iceberg.PrimitiveTypes.TimestampTzNs, true + } + case *arrow.Decimal128Type: + return iceberg.DecimalTypeOf(int(t.Precision), int(t.Scale)), true + case arrow.ExtensionType: + if t.ExtensionName() == "arrow.uuid" { + return iceberg.PrimitiveTypes.UUID, true + } + } + + return nil, false +} + +func cloneStrs(s []string) []string { return append([]string(nil), s...) } + +func joinPath(s []string) string { return strings.Join(s, ".") } + +// normalizedVariantPath builds the spec's RFC-9535 normalized JSON path. +func normalizedVariantPath(fields []string) string { + if len(fields) == 0 { + return "$" + } + + var b strings.Builder + b.WriteByte('$') + for _, f := range fields { + b.WriteString("['") + b.WriteString(rfc9535Escape(f)) + b.WriteString("']") + } + + return b.String() +} + +func rfc9535Escape(name string) string { + if strings.IndexFunc(name, func(r rune) bool { + return r < 0x20 || r == '\'' || r == '\\' + }) < 0 { + return name + } + + var b strings.Builder + b.Grow(len(name) + 4) + for _, r := range name { + switch r { + case '\b': + b.WriteString(`\b`) + case '\t': + b.WriteString(`\t`) + case '\f': + b.WriteString(`\f`) + case '\n': + b.WriteString(`\n`) + case '\r': + b.WriteString(`\r`) + case '\'': + b.WriteString(`\'`) + case '\\': + b.WriteString(`\\`) + default: + if r < 0x20 { + fmt.Fprintf(&b, `\u%04x`, r) + } else { + b.WriteRune(r) + } + } + } + + return b.String() +} + +// variantFieldBound is a shredded field's lower and upper bound (both required). +type variantFieldBound struct { + jsonPath string + icebergType iceberg.PrimitiveType + lower, upper iceberg.Literal +} + +const variantBoundTruncateLen = 16 + +// truncateVariantBound truncates string/binary leaf bounds to variantBoundTruncateLen; a nil upper drops that bound. +func truncateVariantBound(it iceberg.PrimitiveType, lo, hi iceberg.Literal) (iceberg.Literal, iceberg.Literal) { Review Comment: This truncates to a hardcoded `variantBoundTruncateLen = 16`, but the primitive path derives its length from the column's metrics mode — `createStatsAgg(..., statsCol.Mode.Len)` (parquet_files.go:675). So for a column configured `truncate(8)`, `truncate(32)`, or `full`, primitive bounds honor that setting while these variant child bounds stay capped at 16. Could you thread the parent variant column's mode through instead? `collectVariantBounds` already has `statsCols[parentID].Mode` in scope at the call site (parquet_files.go:1132), so passing the mode (or its `Len`) into `truncateVariantBound` and applying: - `Truncate` ⇒ use `Mode.Len` - `Full` ⇒ no truncation (return `lo`/`hi` unchanged) would make variant child bounds consistent with primitive columns. Default `truncate(16)` behavior is unchanged. -- 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]
