[
https://issues.apache.org/jira/browse/FLINK-40319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated FLINK-40319:
-----------------------------------
Labels: pull-request-available (was: )
> Add a raw-bytes accessor for VARIANT string values, and use it to validate
> UTF-8 on the character-string cast and to let a string variant reach a binary
> target
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: FLINK-40319
> URL: https://issues.apache.org/jira/browse/FLINK-40319
> Project: Flink
> Issue Type: Improvement
> Components: API / Core
> Reporter: Ramin Gharib
> Assignee: Ramin Gharib
> Priority: Major
> Labels: pull-request-available
>
> h3. Background
> {\{BinaryVariantUtil}} can return the raw payload of a binary value but not
> of a string value. \{{getBinary}} copies the bytes out, while \{{getString}}
> decodes them in the same step:
> {code:java}
> // getBinary
> return Arrays.copyOfRange(value, start, start + length);
> // getString
> return new String(value, start, length);
> {code}
> Two things follow from there being no undecoded accessor.
> *A character-string cast cannot reject invalid UTF-8 for a string value.*
> FLINK-37925 added that check for a stored \{{BYTES}} value, where
> \{{Variant#getBytes}} hands over the raw bytes. A stored \{{STRING}} value
> has no equivalent, so the cast keeps the lenient decode and substitutes
> \{{U+FFFD}} for malformed input. The value reaching the query is then a
> character the stored bytes never held.
> *A string variant cannot reach a binary target.*
> \{{VariantCastUtils#toBytes}} calls \{{Variant#getBytes}}, which rejects
> every kind other than \{{BYTES}}, so \{{CAST(v AS BYTES)}} fails for a string
> variant regardless of its content. That is inconsistent in two ways. Regular
> SQL allows \{{STRING}} to \{{BINARY}}, and the VARIANT rules already allow
> the mirror direction of \{{BYTES}} to a character string, justified on the
> grounds that a cast renders the way a regular cast of the stored kind would.
> It is also the infallible direction, since encoding a string to UTF-8 always
> succeeds while decoding can fail.
> Together these leave a stored \{{STRING}} value with malformed bytes
> unreachable. The character-string cast silently returns replacement
> characters and the binary cast fails on a kind mismatch, so nothing shows
> what was actually stored.
> h3. Scope
> # Add \{{BinaryVariantUtil#getStringBytes(byte[], int)}}, parsing the
> \{{SHORT_STR}} and \{{LONG_STR}} header exactly as \{{getString}} does and
> returning \{{Arrays.copyOfRange}} as \{{getBinary}} does. Keep the header
> parsing duplicated instead of refactoring \{{getString}} to delegate. Spark
> already duplicates it between \{{getBinary}} and \{{getString}}, and
> delegating would add an array copy to every \{{getString}} call.
> # Add \{{BinaryVariant#getStringBytes()}} guarded by
> \{{checkType(Type.STRING, getType())}}, mirroring \{{getBytes()}}. Do not add
> it to \{{Variant}}, which is \{{@PublicEvolving}}.
> # In \{{VariantCastUtils#toStringValue}}, collapse the \{{STRING}} and
> \{{BYTES}} branches. Both become read the raw bytes, validate, decode. One
> message then covers both, and the advice to cast to \{{BYTES}} becomes true
> for both.
> # In \{{VariantCastUtils#toBytes}}, accept a stored \{{STRING}} value through
> its UTF-8 bytes, and reject every other kind with \{{unsupportedKind}}
> instead of letting \{{VariantTypeException}} escape and be rewrapped.
> h3. Observable changes
> || Expression on a string variant || Before || After ||
> | \{{CAST(v AS STRING)}}, malformed bytes | returns \{{U+FFFD}} | fails,
> reporting the first invalid byte |
> | \{{CAST(v AS BYTES)}} | fails on a kind mismatch | returns the stored bytes
> |
> The second row flips an existing expectation in \{{CastRulesTest}}, which
> asserts that a string variant fails a \{{BYTES}} cast. It becomes a passing
> case, so it should be called out in the commit message rather than looking
> like a test fixup. The prose table of cast targets in
> \{{docs/content/docs/sql/reference/data-types.md}} needs a \{{STRING}} to
> \{{BINARY}} and \{{VARBINARY}} entry. The support matrix in the same file
> already documents VARIANT to \{{BYTES}} as fallible and needs no change.
> h3. Notes
> {\{Variant}} stays untouched, so \{{BinaryVariant}} has to be reached through
> an \{{instanceof}} check in \{{VariantCastUtils}}. That is safe in practice.
> \{{BinaryVariant}} is \{{final}} and the only implementation, and
> \{{VariantType}} permits only \{{Variant}} and \{{BinaryVariant}} as
> conversion classes.
> Do not generalise the accessor to every stored kind. \{{BOOLEAN}} and
> \{{NULL}} carry no payload bytes at all, because the value lives in the
> header type-info bits. A numeric payload is a little-endian detail of the
> variant encoding rather than a portable value. Regular SQL also rejects
> \{{INTEGER}} to \{{BINARY}}, so accepting every kind would make a VARIANT
> cast more permissive than a regular one. For inspecting the encoding of an
> arbitrary kind, \{{BinaryVariant#getValue}} and
> \{{BinaryVariant#getMetadata}} already exist, and a SQL-level equivalent
> belongs in a dedicated function rather than in \{{CAST}}.
> The character-string cast pays one extra array copy per row, since
> \{{getStringBytes}} copies before the validation walk. Validation at ingest
> would make the check redundant and allow removing it.
> h3. Tests
> {\{VariantBuilder}} takes a \{{String}}, so it cannot produce a string value
> with malformed bytes. Build one directly with \{{new BinaryVariant(value,
> metadata)}}, using \{{BinaryVariantUtil#shortStrHeader}} for the header byte
> and a single \{{BinaryVariantUtil#VERSION}} byte as metadata. Cover a
> non-ASCII string round-tripping through both targets, and a malformed string
> value failing the character-string cast while still reaching \{{BYTES}} byte
> for byte.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
