[
https://issues.apache.org/jira/browse/IMPALA-15460?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Zoltán Borók-Nagy resolved IMPALA-15460.
----------------------------------------
Resolution: Fixed
> VARIANT: objects whose values are not stored in field-name order (valid per
> spec; Spark writes them) lose fields: variant_get returns NULL silently,
> SELECT of the column fails
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: IMPALA-15460
> URL: https://issues.apache.org/jira/browse/IMPALA-15460
> Project: IMPALA
> Issue Type: Bug
> Components: Backend
> Reporter: Zoltán Borók-Nagy
> Assignee: Zoltán Borók-Nagy
> Priority: Major
> Labels: impala-iceberg
> Fix For: Impala 5.0.0
>
>
> h3. Problem
> The Parquet Variant spec sorts an object's field ids and field offsets by
> field name, but the values can be stored in any order
> ([VariantEncoding.md|https://github.com/apache/parquet-format/blob/master/VariantEncoding.md],
> "Value Data for Object"): "The field ids and field offsets must be in
> lexicographical order of the corresponding field names in the metadata
> dictionary. However, the actual {{value}} entries do not need to be in any
> particular order. This implies that the {{field_offset}} values may not be
> monotonically increasing." Spark's {{parse_json}} stores the values in
> document order, and {{to_variant_object}} stores them in struct field or map
> entry order.
> Impala uses the next offset in name order, {{offset\[i+1\]}}, as the end of
> field i. If that offset is smaller than {{offset\[i\]}}, Impala treats the
> field as unreadable:
> * {{variant_get}} (typed and untyped) and {{try_variant_get}} return NULL for
> a path that ends at or passes through that field. There is no error and no
> warning, even from the strict {{variant_get}}
> (ff43e6e2cf43e8c9:99b5247900000000). Filters silently drop rows: {{WHERE
> variant_get(v, '$.a.x', 'int') = 2}} returns 0 instead of 1
> (75419747410b1b42:b297002100000000).
> * {{variant_to_json}} returns NULL for the whole value, with {{UDF WARNING:
> Failed to read object field}}.
> * Returning such a VARIANT to the client fails the whole query with {{Failed
> to read object field}}: {{SELECT * FROM t}}
> (4c40575dfff512c3:b40b24a300000000), {{SELECT id, v ... WHERE id = 2}}
> (c74366f497c4edda:cbb0855700000000), {{SELECT variant_get(v, '$.a') ... WHERE
> id = 4}} (c647f976c34c257d:0d88e9bb00000000). See
> {{be/src/service/hs2-util.cc:441-447}} and {{query-result-set.cc:254}}.
> * {{variant_get(v, path, 'string')}} on an object that contains such a field
> ({{'$'}} on row 2) fails with {{UDF ERROR: variant_get: corrupt or unreadable
> variant}} (7645a6436c5f4db7:21d2b20600000000). {{try_variant_get}} returns
> NULL with that warning (0b4db80a99f646e1:faa8daf700000000). See
> {{variant-functions-ir.cc:397-398}}. Valid data is reported as corrupt.
> Only fields whose name-order successor is stored *earlier* fail, so the other
> fields read fine and the wrong answers look plausible. With Spark-written
> data, any object whose keys are not in sorted (byte) order is affected.
> VARIANT is new in 5.0.0, so no release is affected yet; this should be fixed
> before 5.0.0.
> h3. Steps to reproduce
> Spark 4.1.3 + Iceberg 1.11.0 ({{ath}} is the Spark catalog name of the
> Iceberg REST catalog):
> {code:sql}
> CREATE NAMESPACE IF NOT EXISTS ath.zz_jira_variant;
> CREATE TABLE ath.zz_jira_variant.variant_order (id INT, v VARIANT) USING
> iceberg
> TBLPROPERTIES ('format-version'='3');
> INSERT INTO ath.zz_jira_variant.variant_order VALUES
> (1, parse_json('{"a":1,"b":2}')),
> (2, parse_json('{"b":2,"a":1}')),
> (3, parse_json('{"a":1,"c":3,"b":2}')),
> (4, parse_json('{"a":{"y":1,"x":2},"b":0}'));
> {code}
> Impala, reading the same catalog (impalad {{--catalog_config_dir}}):
> {code:sql}
> SELECT id, variant_to_json(v) AS doc, variant_to_json(variant_get(v, '$.a'))
> AS a,
> variant_get(v, '$.b', 'int') AS b, variant_get(v, '$.a.x', 'int') AS
> a_x
> FROM zz_jira_variant.variant_order ORDER BY id;
> {code}
> h3. Expected result
> The Spark answer, using {{to_json}} for {{variant_to_json}}. DuckDB 1.5.5
> ({{read_parquet}} on the same data files) gives the same answer.
> {noformat}
> id | doc | a | b | a_x
> 1 | {"a":1,"b":2} | 1 | 2 | NULL
> 2 | {"a":1,"b":2} | 1 | 2 | NULL
> 3 | {"a":1,"b":2,"c":3} | 1 | 2 | NULL
> 4 | {"a":{"x":2,"y":1},"b":0} | {"x":2,"y":1} | 0 | 2
> {noformat}
> h3. Actual result (master 3d4d47dc30, RELEASE; query
> 4e442488d6e7bfa8:41812d4b00000000)
> {noformat}
> id | doc | a | b | a_x
> 1 | {"a":1,"b":2} | 1 | 2 | NULL
> 2 | NULL | NULL | 2 | NULL
> 3 | NULL | 1 | NULL | NULL
> 4 | NULL | NULL | 0 | NULL
> {noformat}
> The query log has four warnings {{UDF WARNING: Failed to read object field}}.
> {{SELECT count(*) ... WHERE variant_get(v, '$.b', 'int') IS NULL}} returns 1
> (row 3), with no warning (ac475cede0ffbae6:d7c0dffc00000000). The query ids
> in Problem are from a second copy of the table (same statements, identical
> output: f642e4eabe18623a:494755eb00000000). In row 3, "c" is stored out of
> order too but reads fine (484b82cd94a8843a:bd69e7b200000000).
> Row 2 as stored by Spark (read with pyarrow): metadata {{01 02 00 01 02 62
> 61}} (dictionary \["b", "a"\], sorted_strings=0); value {{02 02 01 00 02 00
> 04 0c 02 0c 01}}: header, 2 fields, field ids \[a, b\] (name order), offsets
> \[2, 0, 4\], then the values b=2 (at 0) and a=1 (at 2). For "a", Impala reads
> start=2, end=offset\[1\]=0 and gives up.
> h3. Which shapes fail
> 16 Spark-written test documents: 11 read wrong, 5 right. A field fails
> exactly when {{offset\[i+1\] < offset\[i\]}} (checked with pyarrow); every
> other field reads right.
> * Wrong: reversed keys ({{\{"c":3,"b":2,"a":1\}}}, the spec's own example),
> one key out of place, byte order ({{\{"a":1,"B":2\}}}), nested objects,
> objects inside arrays, 2- and 3-byte field offsets, 300 reversed fields
> ({{is_large}}).
> * Right: sorted keys, one field, the empty object, top-level arrays. Offset
> sizes (1 to 4 bytes) and {{is_large}} do not change the outcome; 4-byte
> offsets were checked separately, with a value over 16 MiB
> (e84c83d93ca74d83:c77bf8a700000000).
> * After {{parse_json(to_json(v))}} (Spark's {{to_json}} emits sorted keys):
> 16/16 right.
> Benchmark (Iceberg Athletics Pole Vault, table {{variant.events_plain}}, 100M
> rows at sf1): 11 of 30 {{plain}} queries are wrong at tiny and sf1; DuckDB
> 1.5.5 (tiny) and StarRocks 4.1.4 (sf1) answer all 30 right, Trino 483 (sf1)
> 28 (2 unsupported). {{plain.q1}}: {{sum(variant_get(payload, '$.duration',
> 'int'))}} is 3,597,278,124 instead of 179,940,024,471
> ({{results/variant/impala5-rest/sf1/}}). {{SELECT id, payload ... WHERE id =
> 0}} fails (df4bc2f51a85930f:e962ca6d00000000), and
> {{variant_to_json(payload)}} is NULL in all 100,000,000 rows
> (3544252c08c50795:4b2a653a00000000). Rows readable through
> {{try_variant_get}} compared with {{get_json_object}} on the JSON copy
> (8248e86cf82ed174:a9d8ddbe00000000, 1846e46de6a44e55:c3ef16a800000000):
> {noformat}
> path VARIANT JSON
> country, event_type, ts_us, user.id, metrics.timing.load_ms 100,000,000
> 100,000,000
> coupon 2,001,260
> 2,001,260
> duration 1,998,857
> 100,000,000
> error.code 0
> 1,998,857
> items[0].qty 0
> 22,999,379
> referrer 0
> 29,994,559
> tags[0] 0
> 74,999,893
> device, metrics.flags.is_bot 0
> 100,000,000
> {noformat}
> "duration" is readable only when an "error" key follows it. For ids <
> 5,000,000, the rows where duration is readable are exactly the rows that have
> "error" (0 mismatches, d641b45f97859c58:f24a3b1a00000000).
> h3. Analysis (be/src/runtime/variant-value.cc at master 3d4d47dc30)
> * {{VariantValue::FieldFromLayout()}} (lines 312-321) reads {{start =
> offset\[index\]}} and {{end = offset\[index + 1\]}}, then line 318: {{if (end
> < start || end > layout.data_len) return false;}}. {{GetFieldByName()}} (line
> 307) and the JSON serializer (lines 658-660, "Failed to read object field")
> both use it.
> * Per the spec, {{offset\[i+1\]}} is where the *next field in name order*
> starts, not where field i ends. The reference reader does not use it:
> parquet-java {{Variant.getFieldAtIndex()}} slices from {{dataStart +
> offset}}, and {{VariantUtil.valueSize()}} takes a value's size from its own
> header.
> * Introduced by IMPALA-15221 (7fe7e1b35a). Its description lists "next_offset
> - field_offset can unsigned-underflow" as a corruption symptom, but for
> objects this is valid data. Before it, IMPALA-15067's {{GetFieldByIndex()}}
> computed the length with unsigned wrap-around (e5727bb717
> {{variant-value.cc:308-314}}); by code reading the values still decoded,
> because every decoder sizes a value from its header. So the regression is in
> SELECT of the column: IMPALA-15052 (480cdc593d, committed 2026-07-29)
> returned such objects until IMPALA-15221 was committed (2026-07-31).
> {{variant_get}} and the {{variant_to_json}} builtin were both added by
> IMPALA-15057 (1e3598ce49), after IMPALA-15221, and never worked on such data.
> * The error is silent. {{GetFieldByName()}} returns false both for "no such
> field" and for "unreadable field". {{NavigatePath()}} (line 440) passes that
> on, and {{Navigate()}} ({{be/src/exprs/variant-functions-ir.cc:77}}) maps it
> to {{SQL_NULL}} ("segment missing").
> * Why tests missed it: both {{BuildObject()}} helpers
> ({{be/src/util/variant-util-test.cc:87}},
> {{be/src/exprs/variant-functions-test.cc:106}}) always store values in
> field-list order. The Trino-written test tables {{trino_variant}} and
> {{trino_nested_variant}} have 19 objects, none with a decreasing offset
> (pyarrow). The {{ToJson}} unit test ({{variant-util-test.cc:356-371}}) even
> builds an object whose field ids are not in name order, which the spec does
> not allow. IMPALA-15221 added a {{ValueCorruptionRejected}} case, "Object
> with non-monotonic field offsets \[5, 0\]"
> ({{variant-util-test.cc:939-941}}). It must stay rejected, but for another
> reason: the field starts at 5, which is the end of the 5-byte data region.
> Keep the case and fix its comment.
> Suggested fix:
> * Add a {{ValueSize(data, avail, &size)}} helper that computes a value's size
> from its header, like parquet-java {{valueSize()}}. Primitives have fixed
> sizes; a short string is 1+len; a long string or binary is 5+len; an object
> or array is its header and tables plus its last offset. Bounds-check the
> result against {{avail}}. Compute header and table sizes in 64 bits, as
> {{ParseObjectLayout()}} does. Reject unknown primitive type ids (21-63), but
> size the types Impala does not render yet (TIMESTAMPTZ, TIME,
> TIMESTAMPTZ_NANOS: 9 bytes; UUID: 17 bytes).
> * In {{FieldFromLayout()}}, check {{start < data_len}} and take the length
> from {{ValueSize(data + start, data_len - start)}}; do not use
> {{offset\[i+1\]}}. The {{start < data_len}} check keeps the \[5, 0\] test
> case rejected.
> * Defensive, optional: size array elements the same way
> ({{ElementFromLayout()}}, lines 379-388, check at 385). Spark writes array
> elements in order, but the spec defines array offsets only as start positions.
> * Report an unreadable field as corrupt (an error from {{variant_get}}, a
> warning from {{try_variant_get}}), not as a missing field.
> {{GetFieldByName()}} and {{NavigatePath()}} need a third result for this,
> because a missing field or a path step into a non-object must still give
> NULL. Land this together with the {{FieldFromLayout()}} change: {{start <
> data_len}} rejects the zero-length field in {{EmptyValueAtPath}}
> ({{be/src/exprs/variant-functions-test.cc:556}}). Today that is an error from
> the strict {{variant_get}}; with the {{FieldFromLayout()}} change alone it
> would become NULL.
> * Tests: a unit test with the row 2 bytes above. Let both {{BuildObject()}}
> helpers store values in a given order, so that {{variant_get}} and
> {{try_variant_get}} are covered too, and make the {{ToJson}} test spec-valid.
> Add a Spark-written Parquet file with reversed, nested, byte-order,
> {{to_variant_object}} and 2-/3-byte-offset objects (see IMPALA-15231).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
