nssalian commented on code in PR #1607:
URL: https://github.com/apache/iceberg-go/pull/1607#discussion_r3899680013
##########
table/evaluators.go:
##########
@@ -868,25 +880,44 @@ func (m *inclusiveMetricsEval) VisitNotNan(t
iceberg.BoundTerm) bool {
return rowsMightMatch
}
-func (m *inclusiveMetricsEval) VisitLess(t iceberg.BoundTerm, lit
iceberg.Literal) bool {
- field := t.Ref().Field()
- fieldID := field.ID
+// boundFor decodes the file bound for term t from raw: a scalar for a
reference, or the
+// variant sub-path value for an extract; ok is false when raw is nil or not
castable.
+func (m *inclusiveMetricsEval) boundFor(t iceberg.BoundTerm, raw []byte)
(iceberg.Literal, bool) {
+ if raw == nil {
+ return nil, false
+ }
- if m.containsNullsOnly(fieldID) || m.containsNansOnly(fieldID) {
- return rowsCannotMatch
+ if ext, ok := t.(iceberg.BoundExtract); ok {
+ lit, found, err := internal.VariantBoundLiteral(raw,
ext.Path(), ext.Type().(iceberg.PrimitiveType))
+ if err != nil {
+ panic(err)
Review Comment:
Done for extracts - `boundFor` now returns `(nil, false)` on a decode error
so pruning stays best-effort, no panic. Left the scalar-reference panic as-is
since it's pre-existing and out of scope for this PR
##########
variant_extract.go:
##########
@@ -0,0 +1,248 @@
+// 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 iceberg
+
+import (
+ "fmt"
+
+ "github.com/apache/arrow-go/v18/parquet/variant"
+ "github.com/google/uuid"
+)
+
+// BoundExtract is a bound variant sub-path term used for metrics pruning and
residual evaluation.
+type BoundExtract interface {
+ BoundTerm
+
+ Path() string
+ // ExtractValue navigates v to this term's path and casts the leaf to
the target type.
+ ExtractValue(v variant.Value) (Literal, bool)
+}
+
+// Extract creates an unbound variant sub-path term for a dotted JSONPath.
+func Extract(ref Reference, path string, typ PrimitiveType) UnboundTerm {
+ return &unboundExtract{ref: ref, path: path, typ: typ}
+}
+
+type unboundExtract struct {
Review Comment:
Now, both `unboundExtract` and `boundExtract` now have `MarshalJSON`
returning `ErrExtractNotSerializable`, so it errors instead of shipping `{}`.
Checked that the marshal sites (expr_json, REST scan-planning, codec) all
propagate it. Asserted in `expr_json_test.go`.
##########
table/variant_residual.go:
##########
@@ -0,0 +1,193 @@
+// 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 table
+
+import (
+ "context"
+ "fmt"
+ "strconv"
+
+ "github.com/apache/arrow-go/v18/arrow"
+ "github.com/apache/arrow-go/v18/arrow/array"
+ "github.com/apache/arrow-go/v18/arrow/compute"
+ "github.com/apache/arrow-go/v18/arrow/extensions"
+ "github.com/apache/arrow-go/v18/arrow/memory"
+ "github.com/apache/iceberg-go"
+ "github.com/google/uuid"
+)
+
+// augmentSchemaWithExtracts returns fileSchema plus one primitive column per
variant extract term.
+func augmentSchemaWithExtracts(fileSchema *iceberg.Schema, cols
[]iceberg.VariantExtractColumn) (*iceberg.Schema, error) {
+ fields := fileSchema.Fields()
+ for _, c := range cols {
+ fields = append(fields, iceberg.NestedField{
+ ID: c.FieldID,
+ Name: c.Name,
+ Type: c.Term.Type().(iceberg.PrimitiveType),
+ })
+ }
+
+ return iceberg.NewSchema(fileSchema.ID, fields...), nil
+}
+
+// buildExtractColumn materializes one variant extract term into a typed Arrow
array over rec.
+func buildExtractColumn(col iceberg.VariantExtractColumn, rec
arrow.RecordBatch, mem memory.Allocator) (arrow.Array, arrow.Field, error) {
+ typ := col.Term.Type().(iceberg.PrimitiveType)
+ dt, err := TypeToArrowType(typ, false, false)
+ if err != nil {
+ return nil, arrow.Field{}, err
+ }
+
+ bldr := array.NewBuilder(mem, dt)
+ defer bldr.Release()
+
+ n := int(rec.NumRows())
+ varIdx := fieldIndexByID(rec.Schema(), col.Term.Ref().Field().ID)
+ varr, _ := columnAt(rec, varIdx).(*extensions.VariantArray)
Review Comment:
Done. A missing column now returns an error, and the `*VariantArray`
assertion uses `, ok` and errors instead of leaving `varr` nil, so it can't
null out the whole batch. Testing is covered by
`TestBuildExtractColumnNameFallback` and
`TestVariantExtractOrAbsentColumnNoPanic`
--
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]
