Spec: add variant type #10831

aihuaxu · 2024-07-31T23:22:06Z

Spec: add variant type

Proposal: https://docs.google.com/document/d/1QjhpG_SVNPZh3anFcpicMQx90ebwjL7rmzFYfUP89Iw/edit

This is to layout the spec for variant type. The specs are placed in Parquet project (see variant spec and shredding spec.

aihuaxu · 2024-07-31T23:37:24Z

cc @rdblue, @RussellSpitzer and @flyrain

format/spec.md

format/variant-shredding-spec.md

RussellSpitzer · 2024-08-01T15:26:42Z

I do want to make sure we don't do a hostile fork here of the spec from Spark so let's make sure we get support from them to move the spec here before we merge. At the same time we should start going through wordings and continue to discuss the specs. I still think that would be easier to do in a public Google Doc though than in Github IMHO.

sfc-gh-aixu · 2024-08-01T16:30:08Z

I do want to make sure we don't do a hostile fork here of the spec from Spark so let's make sure we get support from them to move the spec here before we merge. At the same time we should start going through wordings and continue to discuss the specs. I still think that would be easier to do in a public Google Doc though than in Github IMHO.

Definitely. It's not for merge yet. I'm mostly trying to get the comments in place. Make sense to move that to google doc and link here.

format/spec.md

RussellSpitzer · 2024-10-17T18:28:53Z

This needs some notes in Partition Transforms , I think explicitly we should disallow identity

For Appendix B - We should define something or state explicitly we don't define it for variant.

Appendix C - We'll need some details on the JSON serialization since that's going to have to define some string representations I think

Under Sort Orders we should probably note you cannot sort on a Variant?

Appendix D: Single Value Serialzation needs an entry, we can probably right "Not SUpported" for now, Json needs an entry

RussellSpitzer · 2024-10-17T18:29:37Z

And an entry https://github.com/apache/iceberg/blob/main/format/spec.md#parquet

aihuaxu · 2024-10-18T18:50:58Z

This needs some notes in Partition Transforms , I think explicitly we should disallow identity

For Appendix B - We should define something or state explicitly we don't define it for variant.

Appendix C - We'll need some details on the JSON serialization since that's going to have to define some string representations I think

Under Sort Orders we should probably note you cannot sort on a Variant?

Appendix D: Single Value Serialzation needs an entry, we can probably right "Not SUpported" for now, Json needs an entry

Thanks @RussellSpitzer I missed those sections and just updated.

I mark Partition Transforms, sorting and hashing not supported/allowed for now.
For Appendix C, I think it should be just variant, similar to primitive type, since it's Iceberg schema as I understand the section.

format/spec.md

rdblue · 2024-10-24T21:39:55Z

format/spec.md

@@ -444,6 +449,9 @@ Sorting floating-point numbers should produce the following behavior: `-NaN` < `

 A data or delete file is associated with a sort order by the sort order's id within [a manifest](#manifests). Therefore, the table must declare all the sort orders for lookup. A table could also be configured with a default sort order id, indicating how the new data should be sorted by default. Writers should use this default sort order to sort the data on write, but are not required to if the default order is prohibitively expensive, as it would be for streaming writes.

+Note:
+
+1. The ability to sort `variant` columns and the specific sort order is determined by the engines.


Do we need this? I think anything we don't specify is up to engines already.

OK. I will remove that then. Do we need to call out "Variant values cannot be present in an Iceberg sort order"?

I think we should specifically forbid sort orders containing a variant. I think we actually are underdetermined in the spec here.

We have the following checks in the Reference Implementation

iceberg/api/src/main/java/org/apache/iceberg/SortOrder.java

Lines 301 to 311 in 8a16a41

ValidationException.check(

sourceType != null, "Cannot find source column for sort field: %s", field);

ValidationException.check(

sourceType.isPrimitiveType(),

"Cannot sort by non-primitive source field: %s",

sourceType);

ValidationException.check(

field.transform().canTransform(sourceType),

"Invalid source type %s for transform: %s",

sourceType,

field.transform());

So currently, even though we don't specify this here, you cannot make a sort order with array or map. I think we should explicitly call this out and add variant as well. My real concern here is that we add the ability to sort on something but don't define what that sorting actually looks like.

format/spec.md

rdblue · 2024-10-24T22:35:37Z

Oops. I didn't mean to close this.

aihuaxu · 2024-10-25T20:42:19Z

@aihuaxu, I think there are a couple of things missing:

The Avro appendix should be updated to state that a Variant is stored as a Record with two fields, a required binary metadata and a required binary value. Shredding is not supported in Avro.

The ORC appendix should be updated to state that a Variant is stored as a struct with two fields, a required binary metadata and a required binary value. Type attribute should be iceberg.struct-type=variant. Shredding is not supported in ORC.

Thanks @rdblue I thought we will make changes when we start to work on Avro/ORC. I added that.

I don't have much context for Json conversion. Not sure if we need to add more info.

format/spec.md

RussellSpitzer · 2024-11-01T18:50:47Z

format/spec.md

@@ -1436,6 +1457,7 @@ This serialization scheme is for storing single values as individual binary valu
 | **`struct`**                 | Not supported                                                                                                |
 | **`list`**                   | Not supported                                                                                                |
 | **`map`**                    | Not supported                                                                                                |
+| **`variant`**                | Not supported                                                                                                |


I do agree this should be not-supported for now. Then when shredding is included say something like for Shredded variants only, binary value concatenation of metadata and value + separator byte or something. We can figure that out with the shredding addition though

If we don't include Variant here, then we don't need to include it in the JSON section either.

Seems binary representation is used for lower bound and upper bound and JSON single-value serialization is used for default value. Looks like they are defined independently? But since there is no default value for Variant, i will remove from JSON section.

format/spec.md

rdblue · 2024-11-21T19:38:07Z

format/spec.md

@@ -178,6 +178,21 @@ A **`list`** is a collection of values with some element type. The element field

 A **`map`** is a collection of key-value pairs with a key type and a value type. Both the key field and value field each have an integer id that is unique in the table schema. Map keys are required and map values can be either optional or required. Both map keys and map values may be any type, including nested types.

+#### Semi-structured Types
+
+A **`variant`** is a value that stores semi-structured data. The structure and data types in a variant are not necessarily consistent across rows in a table or data file. The variant type and binary encoding are defined in the [Parquet project](https://github.com/apache/parquet-format/blob/4f208158dba80ff4bff4afaa4441d7270103dff6/VariantEncoding.md). Support for Variant is added in Iceberg v3.


This link should be to main/master rather than a specific sha right?

This I worried about, since aren't we syncing with a specific iteration of the file?

Basically we link to a specific version to be implemented in Iceberg. Then later e.g., when we add additional data types, we should also update here.

format/spec.md

rdblue · 2024-11-21T19:39:57Z

format/spec.md

@@ -444,7 +459,7 @@ Partition field IDs must be reused if an existing partition spec contains an equ

 | Transform name    | Description                                                  | Source types                                                                                              | Result type |
 |-------------------|--------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------|-------------|
-| **`identity`**    | Source value, unmodified                                     | Any                                                                                                       | Source type |
+| **`identity`**    | Source value, unmodified                                     | Any other than `variant`                                                                                  | Source type |


What out "except for"? I think that is more clear than "other than"

Should we also fix the bucket function definition by using an except for list?

Make sense. Let me fix in the following PR.

Should we also fix the bucket function definition by using an except for list?

rdblue

Looks good to me other than a few minor comments.

Proposal: https://docs.google.com/document/d/1QjhpG_SVNPZh3anFcpicMQx90ebwjL7rmzFYfUP89Iw/edit

emkornfield · 2024-11-22T23:10:09Z

format/spec.md

@@ -1154,6 +1169,7 @@ Maps with non-string keys must use an array representation with the `map` logica
 |**`struct`**|`record`||
 |**`list`**|`array`||
 |**`map`**|`array` of key-value records, or `map` when keys are strings (optional).|Array storage must use logical type name `map` and must store elements that are 2-field records. The first field is a non-null key and the second field is the value.|
+|**`variant`**|`record` with `metadata` and `value` fields. `metadata` and `value` must not be assigned field IDs. |Shredding is not supported in Avro.|


we should probably be consistent here and make the note that field IDs should not be assigned to the fields across all formats?

I added the same note for parquet and ORC format. I see ORC use ICEBERG_ID_ATTRIBUTE to track the fieldId but the concept seems to be similar.

github-actions bot added the Specification Issues that may introduce spec changes. label Jul 31, 2024

aihuaxu force-pushed the variant-type-spec branch from b868ea6 to 1a0404b Compare July 31, 2024 23:24

flyrain reviewed Aug 1, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

flyrain reviewed Aug 1, 2024

View reviewed changes

format/variant-shredding-spec.md Outdated Show resolved Hide resolved

aihuaxu force-pushed the variant-type-spec branch from e51c8e6 to 5a8acf1 Compare October 9, 2024 20:59

aihuaxu marked this pull request as ready for review October 9, 2024 20:59

aihuaxu force-pushed the variant-type-spec branch from 5a8acf1 to 408ad2d Compare October 9, 2024 21:03

RussellSpitzer reviewed Oct 10, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

RussellSpitzer reviewed Oct 10, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

RussellSpitzer reviewed Oct 10, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

aihuaxu force-pushed the variant-type-spec branch from 285d009 to f7adbbc Compare October 17, 2024 15:53

aihuaxu requested a review from RussellSpitzer October 17, 2024 15:53

aihuaxu force-pushed the variant-type-spec branch from f7adbbc to 3e91ce9 Compare October 17, 2024 16:05

aihuaxu requested review from flyrain and sfc-gh-aixu October 17, 2024 17:22

aihuaxu force-pushed the variant-type-spec branch from 3e91ce9 to 0bc975e Compare October 18, 2024 18:43

RussellSpitzer reviewed Oct 18, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

flyrain reviewed Oct 18, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

aihuaxu requested review from RussellSpitzer and flyrain October 21, 2024 18:23

aihuaxu force-pushed the variant-type-spec branch from 6673520 to 3aabac4 Compare October 21, 2024 20:46

rdblue reviewed Oct 24, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

rdblue reviewed Oct 24, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

aihuaxu force-pushed the variant-type-spec branch from 636611f to 7023543 Compare October 25, 2024 20:31

aihuaxu requested a review from rdblue October 25, 2024 20:37

aihuaxu force-pushed the variant-type-spec branch 2 times, most recently from d953b6e to 67df611 Compare October 29, 2024 06:24

sfc-gh-rspitzer reviewed Nov 1, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

RussellSpitzer reviewed Nov 1, 2024

View reviewed changes

format/spec.md Show resolved Hide resolved

RussellSpitzer reviewed Nov 1, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

aihuaxu force-pushed the variant-type-spec branch 2 times, most recently from c8f9e7e to 0550fcf Compare November 5, 2024 17:58

aihuaxu requested review from RussellSpitzer and sfc-gh-rspitzer November 5, 2024 17:59

rdblue reviewed Nov 21, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

rdblue reviewed Nov 21, 2024

View reviewed changes

format/spec.md Outdated Show resolved Hide resolved

rdblue reviewed Nov 21, 2024

View reviewed changes

rdblue approved these changes Nov 21, 2024

View reviewed changes

sfc-gh-aixu and others added 5 commits November 22, 2024 09:23

Spec: add variant type

ac95432

Proposal: https://docs.google.com/document/d/1QjhpG_SVNPZh3anFcpicMQx90ebwjL7rmzFYfUP89Iw/edit

Address comments

1c395ba

Update sorting/bucketing/json sections for variant

8be7de1

Update sort info

ab122ea

Update the Variant description and add Avro/ORC type mappings

40c3714

aihuaxu force-pushed the variant-type-spec branch from 0550fcf to ad3a14b Compare November 22, 2024 19:12

emkornfield reviewed Nov 22, 2024

View reviewed changes

aihuaxu force-pushed the variant-type-spec branch from ad3a14b to cd1e8d3 Compare November 24, 2024 04:57

Address comments

ab3b0e1

aihuaxu force-pushed the variant-type-spec branch from cd1e8d3 to ab3b0e1 Compare November 24, 2024 05:20

aihuaxu requested a review from emkornfield November 24, 2024 05:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spec: add variant type #10831

Spec: add variant type #10831

aihuaxu commented Jul 31, 2024 •

edited

Loading

aihuaxu commented Jul 31, 2024

RussellSpitzer commented Aug 1, 2024

sfc-gh-aixu commented Aug 1, 2024

RussellSpitzer commented Oct 17, 2024

RussellSpitzer commented Oct 17, 2024

aihuaxu commented Oct 18, 2024

rdblue Oct 24, 2024

aihuaxu Oct 25, 2024

RussellSpitzer Nov 1, 2024

rdblue commented Oct 24, 2024

aihuaxu commented Oct 25, 2024

RussellSpitzer Nov 1, 2024 •

edited

Loading

rdblue Nov 21, 2024

aihuaxu Nov 22, 2024

rdblue Nov 21, 2024

RussellSpitzer Nov 21, 2024

aihuaxu Nov 22, 2024

rdblue Nov 21, 2024

rdblue Nov 21, 2024

aihuaxu Nov 22, 2024

rdblue left a comment

emkornfield Nov 22, 2024

aihuaxu Nov 24, 2024

	ValidationException.check(
	sourceType != null, "Cannot find source column for sort field: %s", field);
	ValidationException.check(
	sourceType.isPrimitiveType(),
	"Cannot sort by non-primitive source field: %s",
	sourceType);
	ValidationException.check(
	field.transform().canTransform(sourceType),
	"Invalid source type %s for transform: %s",
	sourceType,
	field.transform());

Spec: add variant type #10831

Are you sure you want to change the base?

Spec: add variant type #10831

Conversation

aihuaxu commented Jul 31, 2024 • edited Loading

aihuaxu commented Jul 31, 2024

RussellSpitzer commented Aug 1, 2024

sfc-gh-aixu commented Aug 1, 2024

RussellSpitzer commented Oct 17, 2024

RussellSpitzer commented Oct 17, 2024

aihuaxu commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdblue commented Oct 24, 2024

aihuaxu commented Oct 25, 2024

RussellSpitzer Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdblue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aihuaxu commented Jul 31, 2024 •

edited

Loading

RussellSpitzer Nov 1, 2024 •

edited

Loading