laskoviymishka commented on code in PR #2045: URL: https://github.com/apache/iceberg-go/pull/2045#discussion_r4075300471
########## website/src/SUMMARY.md: ########## @@ -24,6 +24,7 @@ - [Install](./install.md) - [Getting Started](./getting-started.md) - [Configuration](./configuration.md) +- [Variant Type](./variant.md) Review Comment: This lands the Variant Type entry at the top level between Configuration and CLI, but it's really an API/data-type concept page and it cross-links heavily into Expression DSL and Row Filter Syntax, which both live nested under API. I'd move it into the API section next to those two. Intentional, or just where it fell? wdyt? ########## website/src/feature-status.md: ########## @@ -49,8 +49,9 @@ All V1 features are supported. V1 is the format-version baseline. | Default values (`initial-default`, `write-default`) | Supported | | Row lineage (`_row_id`, `_last_updated_sequence_number`) | Supported | | Encryption keys in metadata | Supported | -| Variant type, non-shredded | Supported (PR [#932](https://github.com/apache/iceberg-go/pull/932); umbrella [#929](https://github.com/apache/iceberg-go/issues/929)) | -| Variant type, shredded reader / writer | In progress ([#986](https://github.com/apache/iceberg-go/issues/986), [#987](https://github.com/apache/iceberg-go/issues/987)) | +| Variant type, non-shredded | Supported | Review Comment: We're dropping the PR/issue refs from both variant rows here (#932/#929 off non-shredded, #986/#987 off shredded), but the other Supported rows in this table keep theirs: the sort-order row still cites its PR. Since the page intro says it's kept in sync with README and points at issues for tracking, I'd keep the refs or swap in the final merge PRs. ########## website/src/feature-status.md: ########## @@ -49,8 +49,9 @@ All V1 features are supported. V1 is the format-version baseline. | Default values (`initial-default`, `write-default`) | Supported | | Row lineage (`_row_id`, `_last_updated_sequence_number`) | Supported | | Encryption keys in metadata | Supported | -| Variant type, non-shredded | Supported (PR [#932](https://github.com/apache/iceberg-go/pull/932); umbrella [#929](https://github.com/apache/iceberg-go/issues/929)) | -| Variant type, shredded reader / writer | In progress ([#986](https://github.com/apache/iceberg-go/issues/986), [#987](https://github.com/apache/iceberg-go/issues/987)) | +| Variant type, non-shredded | Supported | +| Variant type, shredded reader / writer | Supported | +| Variant predicate pushdown / field extract | Supported | Review Comment: "predicate pushdown" usually means file/row-group pruning that cuts I/O, but Extract predicates are per-row residuals, and the Constraints section on the variant page says exactly that (stats and bloom pruning skip them). Labeling this row "predicate pushdown" contradicts that page. I'd rename it to something like "Variant field-extract filtering (per-row residual)". ########## website/src/variant.md: ########## @@ -0,0 +1,121 @@ +<!-- + ~ 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. +--> + +# Variant Type + +`variant` is the Iceberg v3 semi-structured type: a single self-describing column +that holds arbitrary JSON-like values (objects, arrays, and scalars) without a +fixed schema. iceberg-go supports both non-shredded and shredded variants, and can +push predicates down into individual variant fields. + +## Reading and writing + +A variant column is declared with `iceberg.VariantType{}` in the table schema and +maps to the Arrow `variant` extension type (see [API](./api.md)). Non-shredded +variant values need no extra configuration: write an Arrow record whose variant +column uses the extension type, and read it back the same way. + +Build the column with a variant builder and write it like any other Arrow column +(the table must be format version 3): + +```go +import ( + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/extensions" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/parquet/variant" +) + +// tbl was created with table.PropertyFormatVersion = "3" and this schema. +schema := iceberg.NewSchema(0, + iceberg.NestedField{ID: 1, Name: "payload", Type: iceberg.VariantType{}}, +) +arrowSchema, _ := table.SchemaToArrowSchema(schema, nil, true, false) + +bldr := extensions.NewVariantBuilder(memory.DefaultAllocator, extensions.NewDefaultVariantType()) +defer bldr.Release() + +var vb variant.Builder +_ = vb.Append(map[string]any{"x": int64(320), "target": "button-submit"}) +val, _ := vb.Build() +bldr.Append(val) + +col := bldr.NewArray() +defer col.Release() +rec := array.NewRecord(arrowSchema, []arrow.Array{col}, 1) +defer rec.Release() + +tx := tbl.NewTransaction() +_ = tx.AppendTable(ctx, array.NewTableFromRecords(arrowSchema, []arrow.Record{rec}), 1024, nil) +tbl, _ = tx.Commit(ctx) +``` + +A normal scan reads it back; the variant column comes back as an `*extensions.VariantArray`: + +```go +result, _ := tbl.Scan().ToArrowTable(ctx) +defer result.Release() + +variants := result.Column(0).Data().Chunk(0).(*extensions.VariantArray) +v, _ := variants.Value(0) +fmt.Println(v.Value()) // variant.ObjectValue for the object above +``` + +## Shredding + +Shredding stores the fields of a variant as typed Parquet sub-columns instead of a +single opaque blob. This lets scans read and filter a field as a native column +rather than decoding every value. Shredding is opt-in and configured with two write +properties (see [Configuration](./configuration.md#parquet-writer)): + +- `write.parquet.shred-variants` - enable shredding of top-level variant columns + (default `false`). +- `write.parquet.variant-inference-buffer-size` - rows buffered per file to infer + the shredding schema (default `100`). + +The reader accepts both shredded and non-shredded data; no read-side +configuration is required. + +## Filtering on variant fields + +`iceberg.Extract(ref, path, typ)` builds a term that navigates a dotted JSONPath Review Comment: "dotted JSONPath" oversells what the parser actually takes. `variant_path.go` accepts member selectors only, dot (`$.user.age`) or quoted bracket (`$['user']['age']`) notation, and rejects array indexing (`$.items[0]`), wildcards, filters, and recursive descent at bind time. Anyone who knows JSONPath will reach for `events[0]` first and hit a runtime error with nothing in the docs to explain it. I'd reword to something like "a member-selector path (`$.field.sub` or `$['field']['sub']`); array indexing and wildcards aren't supported", and make the same edit in the row-filter-syntax section (line 93) and the expression-dsl bullet (line 59), since all three carry the identical phrasing. ########## website/src/variant.md: ########## @@ -0,0 +1,121 @@ +<!-- + ~ 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. +--> + +# Variant Type + +`variant` is the Iceberg v3 semi-structured type: a single self-describing column +that holds arbitrary JSON-like values (objects, arrays, and scalars) without a +fixed schema. iceberg-go supports both non-shredded and shredded variants, and can +push predicates down into individual variant fields. + +## Reading and writing + +A variant column is declared with `iceberg.VariantType{}` in the table schema and +maps to the Arrow `variant` extension type (see [API](./api.md)). Non-shredded +variant values need no extra configuration: write an Arrow record whose variant +column uses the extension type, and read it back the same way. + +Build the column with a variant builder and write it like any other Arrow column +(the table must be format version 3): + +```go +import ( + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/extensions" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/parquet/variant" +) + +// tbl was created with table.PropertyFormatVersion = "3" and this schema. +schema := iceberg.NewSchema(0, + iceberg.NestedField{ID: 1, Name: "payload", Type: iceberg.VariantType{}}, +) +arrowSchema, _ := table.SchemaToArrowSchema(schema, nil, true, false) + +bldr := extensions.NewVariantBuilder(memory.DefaultAllocator, extensions.NewDefaultVariantType()) +defer bldr.Release() + +var vb variant.Builder +_ = vb.Append(map[string]any{"x": int64(320), "target": "button-submit"}) +val, _ := vb.Build() +bldr.Append(val) + +col := bldr.NewArray() +defer col.Release() +rec := array.NewRecord(arrowSchema, []arrow.Array{col}, 1) +defer rec.Release() + +tx := tbl.NewTransaction() +_ = tx.AppendTable(ctx, array.NewTableFromRecords(arrowSchema, []arrow.Record{rec}), 1024, nil) +tbl, _ = tx.Commit(ctx) +``` + +A normal scan reads it back; the variant column comes back as an `*extensions.VariantArray`: + +```go +result, _ := tbl.Scan().ToArrowTable(ctx) +defer result.Release() + +variants := result.Column(0).Data().Chunk(0).(*extensions.VariantArray) +v, _ := variants.Value(0) +fmt.Println(v.Value()) // variant.ObjectValue for the object above +``` + +## Shredding + +Shredding stores the fields of a variant as typed Parquet sub-columns instead of a +single opaque blob. This lets scans read and filter a field as a native column +rather than decoding every value. Shredding is opt-in and configured with two write +properties (see [Configuration](./configuration.md#parquet-writer)): + +- `write.parquet.shred-variants` - enable shredding of top-level variant columns + (default `false`). +- `write.parquet.variant-inference-buffer-size` - rows buffered per file to infer + the shredding schema (default `100`). + +The reader accepts both shredded and non-shredded data; no read-side +configuration is required. + +## Filtering on variant fields + +`iceberg.Extract(ref, path, typ)` builds a term that navigates a dotted JSONPath +into a variant column and casts the leaf to `typ`. It plugs into the same predicate +builders as `iceberg.Reference`: + +```go +filter := iceberg.EqualTo( + iceberg.Extract(iceberg.Reference("payload"), "$.user.age", iceberg.PrimitiveTypes.Int64), + int64(30), +) +// filter is a BooleanExpression; pass it to Scan().WithRowFilter(filter). Review Comment: `(*Scan)` has no `WithRowFilter` method; `table.WithRowFilter` is a standalone `ScanOption` you pass into `Table.Scan(...)`, not something you chain on the scan result. The existing row-filter-syntax page shows the right shape. I'd change the comment to `// pass it to tbl.Scan(table.WithRowFilter(filter))`. ########## website/src/variant.md: ########## @@ -0,0 +1,121 @@ +<!-- + ~ 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. +--> + +# Variant Type + +`variant` is the Iceberg v3 semi-structured type: a single self-describing column +that holds arbitrary JSON-like values (objects, arrays, and scalars) without a +fixed schema. iceberg-go supports both non-shredded and shredded variants, and can +push predicates down into individual variant fields. + +## Reading and writing + +A variant column is declared with `iceberg.VariantType{}` in the table schema and +maps to the Arrow `variant` extension type (see [API](./api.md)). Non-shredded +variant values need no extra configuration: write an Arrow record whose variant +column uses the extension type, and read it back the same way. + +Build the column with a variant builder and write it like any other Arrow column +(the table must be format version 3): + +```go +import ( + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/extensions" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/parquet/variant" +) + +// tbl was created with table.PropertyFormatVersion = "3" and this schema. +schema := iceberg.NewSchema(0, + iceberg.NestedField{ID: 1, Name: "payload", Type: iceberg.VariantType{}}, +) +arrowSchema, _ := table.SchemaToArrowSchema(schema, nil, true, false) + +bldr := extensions.NewVariantBuilder(memory.DefaultAllocator, extensions.NewDefaultVariantType()) +defer bldr.Release() + +var vb variant.Builder +_ = vb.Append(map[string]any{"x": int64(320), "target": "button-submit"}) +val, _ := vb.Build() +bldr.Append(val) + +col := bldr.NewArray() +defer col.Release() +rec := array.NewRecord(arrowSchema, []arrow.Array{col}, 1) +defer rec.Release() + +tx := tbl.NewTransaction() +_ = tx.AppendTable(ctx, array.NewTableFromRecords(arrowSchema, []arrow.Record{rec}), 1024, nil) Review Comment: Small thing while we're here: `array.NewTableFromRecords` is called inline as an arg, so the returned table is never released, while everything else in the snippet is properly `defer`-released. I'd pull it into a variable and `defer t.Release()` so the example models the right cleanup. ########## website/src/variant.md: ########## @@ -0,0 +1,121 @@ +<!-- + ~ 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. +--> + +# Variant Type + +`variant` is the Iceberg v3 semi-structured type: a single self-describing column +that holds arbitrary JSON-like values (objects, arrays, and scalars) without a +fixed schema. iceberg-go supports both non-shredded and shredded variants, and can +push predicates down into individual variant fields. + +## Reading and writing + +A variant column is declared with `iceberg.VariantType{}` in the table schema and +maps to the Arrow `variant` extension type (see [API](./api.md)). Non-shredded +variant values need no extra configuration: write an Arrow record whose variant +column uses the extension type, and read it back the same way. + +Build the column with a variant builder and write it like any other Arrow column +(the table must be format version 3): + +```go +import ( + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/extensions" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/parquet/variant" +) + +// tbl was created with table.PropertyFormatVersion = "3" and this schema. +schema := iceberg.NewSchema(0, + iceberg.NestedField{ID: 1, Name: "payload", Type: iceberg.VariantType{}}, +) +arrowSchema, _ := table.SchemaToArrowSchema(schema, nil, true, false) + +bldr := extensions.NewVariantBuilder(memory.DefaultAllocator, extensions.NewDefaultVariantType()) +defer bldr.Release() + +var vb variant.Builder +_ = vb.Append(map[string]any{"x": int64(320), "target": "button-submit"}) +val, _ := vb.Build() +bldr.Append(val) + +col := bldr.NewArray() +defer col.Release() +rec := array.NewRecord(arrowSchema, []arrow.Array{col}, 1) +defer rec.Release() + +tx := tbl.NewTransaction() +_ = tx.AppendTable(ctx, array.NewTableFromRecords(arrowSchema, []arrow.Record{rec}), 1024, nil) +tbl, _ = tx.Commit(ctx) +``` + +A normal scan reads it back; the variant column comes back as an `*extensions.VariantArray`: + +```go +result, _ := tbl.Scan().ToArrowTable(ctx) +defer result.Release() + +variants := result.Column(0).Data().Chunk(0).(*extensions.VariantArray) +v, _ := variants.Value(0) +fmt.Println(v.Value()) // variant.ObjectValue for the object above +``` + +## Shredding + +Shredding stores the fields of a variant as typed Parquet sub-columns instead of a +single opaque blob. This lets scans read and filter a field as a native column +rather than decoding every value. Shredding is opt-in and configured with two write +properties (see [Configuration](./configuration.md#parquet-writer)): + +- `write.parquet.shred-variants` - enable shredding of top-level variant columns + (default `false`). +- `write.parquet.variant-inference-buffer-size` - rows buffered per file to infer + the shredding schema (default `100`). + +The reader accepts both shredded and non-shredded data; no read-side +configuration is required. + +## Filtering on variant fields + +`iceberg.Extract(ref, path, typ)` builds a term that navigates a dotted JSONPath +into a variant column and casts the leaf to `typ`. It plugs into the same predicate +builders as `iceberg.Reference`: + +```go +filter := iceberg.EqualTo( + iceberg.Extract(iceberg.Reference("payload"), "$.user.age", iceberg.PrimitiveTypes.Int64), + int64(30), +) +// filter is a BooleanExpression; pass it to Scan().WithRowFilter(filter). +``` + +See [Row Filter Syntax](./row-filter-syntax.md#variant-extraction) for the supported +target types and caveats. + +## Constraints + +- A variant column cannot be a partition source or an identity-transform source. +- Extract predicates are evaluated as per-row residual filters. Row-group stats and + bloom pruning skip them; comparison pruning against shredded variant bounds is + best-effort. +- Extract terms are DSL-only: there is no string row-filter form, and they are not + REST-serializable, so they are dropped from any server-side pushed filter and Review Comment: I think this caveat describes behavior that doesn't exist. There's no "partial filter pushed to the server, Extract dropped and run locally" path. Under the default local planning mode there's no server filter at all; the Extract term is just a local residual. With remote scan planning (`ScanPlanningRemote`, or `ScanPlanningAuto` against a capable server) `marshalScanFilter` marshals the bound expression, `boundExtract.MarshalJSON` returns `ErrExtractNotSerializable`, and the whole plan request fails, it isn't silently dropped. I'd reword to say Extract terms run as local residuals under local planning (the default), and that an Extract-containing filter with remote planning surfaces a serialization error rather than being dropped. This same text is duplicated verbatim in the row-filter-syntax caveats (line 107), so I'd fix both, or better, keep the authoritative copy here and have that page point at `variant.md#constraints` so they can't drift. ########## website/src/expression-dsl.md: ########## @@ -56,6 +56,7 @@ A *term* is the left-hand side of a predicate. Iceberg-go has two flavors: - `Reference("column_name")` - an unbound term that names a column (`exprs.go:373`). Typing happens at bind time, when the expression is matched against a schema. This is what you almost always want. - `BoundReference` - the resolved form, produced by `Reference.Bind(schema, caseSensitive)` (`exprs.go:389`). You only encounter these when writing custom expression visitors. +- `Extract(ref, path, type)` - a variant sub-path term that navigates a dotted JSONPath into a `variant` column and casts the leaf to `type` (`variant_extract.go`). Use it inside any predicate builder to filter on a field within a variant. See [Row Filter Syntax](./row-filter-syntax.md#variant-extraction). Review Comment: `type` is a reserved word in Go, and the actual param is `typ` (`variant_extract.go`); the other two doc locations already use `typ`. I'd change both spots in this file (this bullet and the table row on line 130) to `Extract(ref, path, typ)` so a reader can't accidentally copy `type` as an identifier. ########## website/src/variant.md: ########## @@ -0,0 +1,121 @@ +<!-- + ~ 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. +--> + +# Variant Type + +`variant` is the Iceberg v3 semi-structured type: a single self-describing column +that holds arbitrary JSON-like values (objects, arrays, and scalars) without a +fixed schema. iceberg-go supports both non-shredded and shredded variants, and can +push predicates down into individual variant fields. + +## Reading and writing + +A variant column is declared with `iceberg.VariantType{}` in the table schema and +maps to the Arrow `variant` extension type (see [API](./api.md)). Non-shredded +variant values need no extra configuration: write an Arrow record whose variant +column uses the extension type, and read it back the same way. + +Build the column with a variant builder and write it like any other Arrow column +(the table must be format version 3): + +```go +import ( + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/extensions" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/parquet/variant" Review Comment: This import block only lists the arrow-go packages, but the snippet body uses `iceberg.NewSchema`, `iceberg.NestedField`, `iceberg.VariantType{}`, and `table.SchemaToArrowSchema`, so it won't compile as written. This is the most prominent example on the page, so I'd add `github.com/apache/iceberg-go` and `github.com/apache/iceberg-go/table` here (or mark the block a fragment with a `// ...`). -- 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]
