Hi Yunhong, +1(binding) Thank you for working on this!
-- Anton ср, 8 июл. 2026 г. в 06:05, Nicholas Jiang <[email protected]>: > > +1 (binding) > > Thanks Yunhong for driving this forward. I went through the proposal and > also cross-checked it against the current type system and lake integration. > Overall this is a well-designed FIP: following the Parquet Variant binary > encoding, keeping metadata/value as the canonical form, and inferring the > shredding layout autonomously per-Writer is the right direction, and it > aligns Fluss with the Spark 4.0 / Flink 2.1 / Paimon Variant ecosystems. > > My +1 stands. Below are a few implementation-phase notes and questions, > none of which are blocking — mostly about making the v1 scope boundaries > explicit so reviewers of the follow-up PRs know what to expect. > > 1. Row-format coverage beyond Arrow (the main one worth pinning down). > The FIP describes the Arrow columnar path in detail — the > metadata / value / typed_value StructVector, ArrowVariantWriter and > ArrowVariantColumnVector — which is the log-table storage format. > However, Fluss stores primary-key / KV table values in the Compacted > row format (KvFormat.COMPACTED), and also has the Indexed row format; > each has its own field reader/writer that switches over DataTypeRoot, > and InternalRow would gain getVariant(). Could you clarify whether > VARIANT is supported in primary-key tables in v1, or scoped to log > tables first? If it is log-only for now, gating it in > TableDescriptorValidation (and excluding it from primary/partition keys, > like the existing PARTITION_KEY_SUPPORTED_TYPES check) plus a note in > the docs would make the boundary explicit. If it is meant to work > everywhere, then CompactedRowWriter/Reader and IndexedRowWriter/Reader > need the VARIANT path too, and it would be good to state how a Variant > is laid out in a row-oriented (non-Arrow) encoding, since shredding is > inherently columnar. > > 2. Type serde and metadata compatibility. > Because DataTypeRoot intentionally avoids default fallbacks, adding > VARIANT will surface every switch that needs a new case > (DataTypeJsonSerde ser/deser, InternalRow.getDataClass / > createFieldGetter, etc.). Could the FIP confirm an explicit > DataTypeJsonSerde round-trip test for VARIANT? That is the format > persisted in table metadata, so it is what guarantees a table created > on a newer version stays readable and that schema evolution is > forward/backward compatible. > > 3. Lake integration sequencing. > The FIP defers Paimon to a follow-up and Iceberg/Lance to future work, > which is reasonable. For context: today FlussDataTypeToIcebergDataType > already throws UnsupportedOperationException for the constructed types > (Array/Map/Row), and FlussRowAsPaimonRow.getVariant() is currently a > stub that throws. So until those land, a VARIANT column would fail on > the tiering/lake path rather than degrade gracefully. Could we make the > v1 behavior explicit — e.g. a clear "VARIANT is not yet supported for > lake tiering to X" error at table-creation or tiering time, rather than > a late UnsupportedOperationException deep in the writer? > > 4. Predicate/filter pushdown scope. > The projection pushdown via "variant_col:field[::type]" and the > PbVariantFieldProjection wire field are nicely specified. Is filter > pushdown on shredded fields (through LeafPredicate / PredicateBuilder) > in scope for v1, or explicitly future work? Column pruning without > filter pushdown is already a solid win, so I am fine either way — just > worth stating so it is not assumed. > > 5. Shredding convergence and observability. > The FIP already acknowledges that independent Writers can produce > heterogeneous typed_value layouts and that tiering must tolerate mixed > batches. Two follow-up thoughts: (a) is there a plan for cross-Writer > schema convergence, or is per-Writer divergence acceptable indefinitely? > and (b) since inference is a black box in v1, some observability into > the active shredding schema (a system table or metric) would help > operators reason about why a field is or isn't shredded. Both are fine > as future enhancements; flagging for the roadmap. > > Minor: the FIP notes Arrow 15.0.0 is sufficient with 19.0 optional — that > matches the current arrow.version (15.0.0) in the build, so no dependency > bump is required for v1. > > Thanks again for the thorough proposal — looking forward to it. > > Best, > Nicholas Jiang > > On 2026/07/06 03:57:11 yunhong Zheng wrote: > > Hi, all. > > > > I'd like to start a vote on FIP-36: Support Variant Type and Shredding > > [1]. You can find the discussion on it in here [2]. The vote will last > > for at least 72 hours unless there is objection or insufficient votes. > > > > [1] > > https://cwiki.apache.org/confluence/spaces/FLUSS/pages/406623649/FIP-36+Support+Variant+Type+and+Shredding > > [2] https://lists.apache.org/thread/m4wrtxoxrkg0zbgoy7p1yr3o15onylcm > > > > > > Best regards, > > Yunhong > >
