[
https://issues.apache.org/jira/browse/FLINK-40319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ramin Gharib updated FLINK-40319:
---------------------------------
Description:
h3. Problem
{{CAST(v AS BYTES)}} fails for a string variant, whatever the string holds.
{{VariantCastUtils#toBytes}} calls {{Variant#getBytes}}, which accepts only the
{{BYTES}} kind. For {{PARSE_JSON('"abc"')}} the getter throws a
{{VariantTypeException}} with the message {{Expected type BYTES but got
STRING}}.
This is a problem for three reasons:
* Regular SQL allows {{STRING}} to {{BINARY}}. VARIANT already allows the
mirror direction, {{BYTES}} to a character string. Encoding a string to UTF-8
always succeeds.
* JSON has no binary type, so {{PARSE_JSON}} never produces {{BYTES}}. A
variant built from JSON can never reach a binary target, such as a
{{MAP<STRING, BYTES>}} column.
* Every other kind fails the same way, with the getter's
{{VariantTypeException}}. Most other readers, such as the numeric ones, throw a
{{TableRuntimeException}} built by {{VariantCastUtils#unsupportedKind}}
instead. Its message names the cast and the target type.
h3. Proposal
Change only {{VariantCastUtils#toBytes}}:
# Accept a stored {{STRING}} value through
{{variant.getString().getBytes(StandardCharsets.UTF_8)}}, then pad or truncate
it like a stored {{BYTES}} value.
# Reject every other kind with {{unsupportedKind}}.
{{Variant}}, {{BinaryVariant}} and {{BinaryVariantUtil}} stay unchanged.
h3. Observable changes
||Expression||Before||After||
|{{CAST(PARSE_JSON('"abc"') AS BYTES)}}|{{VariantTypeException}}|{{x'616263'}}|
|{{CAST(PARSE_JSON('"abc"') AS
BINARY(2))}}|{{VariantTypeException}}|{{x'6162'}}|
|{{TRY_CAST(PARSE_JSON('"abc"') AS BYTES)}}|{{NULL}}|{{x'616263'}}|
|{{CAST(PARSE_JSON('true') AS
BYTES)}}|{{VariantTypeException}}|{{TableRuntimeException}} with the
{{unsupportedKind}} message|
The first row flips an existing expectation in {{CastRulesTest}}, which asserts
that {{VARIANT_BUILDER.of("foo")}} fails a {{BYTES}} cast. The commit message
calls this out. The table of cast targets in
{{docs/content/docs/sql/reference/data-types.md}} gets a {{STRING}} to
{{BINARY}} and {{VARBINARY}} entry.
h3. Limits
A stored {{STRING}} with an invalid UTF-8 payload loses its bytes.
{{getString}} replaces malformed input with U+FFFD, so the cast returns {{EF BF
BD}} in its place. {{CAST(v AS STRING)}} makes the same substitution today.
Such a value can't come from the builders, which take a Java {{String}}, or
from {{PARSE_JSON}}, which rejects invalid UTF-8. It can only come from bytes
wrapped without validation, such as the binary encoding of the raw format from
FLINK-41323. Validating string payloads at ingest, as FLINK-40318 proposes,
would close this gap. A test pins the substitution.
h3. Out of scope
* A raw-bytes accessor for string values, and UTF-8 validation on the
character-string cast. Ingest validation would make both unnecessary.
* The {{BOOLEAN}} reader and a malformed variant. FLINK-41301 covers them. The
{{DATE}} reader already checks the kind since FLINK-41281.
h3. Tests
* In {{FromVariantConverterTest}}, a non-ASCII string variant reaches
{{BYTES}}, {{VARBINARY(3)}} and {{BINARY(8)}} as its UTF-8 bytes, truncated or
padded like a stored {{BYTES}} value. A string payload {{C3 28}} comes out as
{{EF BF BD 28}}.
* In {{FromVariantConverterTest}}, a boolean, an integer and an object variant
fail a binary cast with a {{TableRuntimeException}}. {{CastRulesTest}} can't
show the exception type, because {{CastExecutor}} wraps any {{Throwable}} in a
{{TableRuntimeException}}.
* In {{CastRulesTest}}, the string case of the {{BYTES}} cast passes.
* In {{CastFunctionITCase}}, {{CAST}} of a string variant to {{BYTES}} returns
its bytes. {{CAST}} of a boolean one fails with the {{unsupportedKind}}
message, and {{TRY_CAST}} returns {{NULL}}.
was:
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.
Summary: Allow CAST from a VARIANT string value to BINARY and VARBINARY
(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)
> Allow CAST from a VARIANT string value to BINARY and VARBINARY
> --------------------------------------------------------------
>
> 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. Problem
> {{CAST(v AS BYTES)}} fails for a string variant, whatever the string holds.
> {{VariantCastUtils#toBytes}} calls {{Variant#getBytes}}, which accepts only
> the {{BYTES}} kind. For {{PARSE_JSON('"abc"')}} the getter throws a
> {{VariantTypeException}} with the message {{Expected type BYTES but got
> STRING}}.
> This is a problem for three reasons:
> * Regular SQL allows {{STRING}} to {{BINARY}}. VARIANT already allows the
> mirror direction, {{BYTES}} to a character string. Encoding a string to UTF-8
> always succeeds.
> * JSON has no binary type, so {{PARSE_JSON}} never produces {{BYTES}}. A
> variant built from JSON can never reach a binary target, such as a
> {{MAP<STRING, BYTES>}} column.
> * Every other kind fails the same way, with the getter's
> {{VariantTypeException}}. Most other readers, such as the numeric ones, throw
> a {{TableRuntimeException}} built by {{VariantCastUtils#unsupportedKind}}
> instead. Its message names the cast and the target type.
> h3. Proposal
> Change only {{VariantCastUtils#toBytes}}:
> # Accept a stored {{STRING}} value through
> {{variant.getString().getBytes(StandardCharsets.UTF_8)}}, then pad or
> truncate it like a stored {{BYTES}} value.
> # Reject every other kind with {{unsupportedKind}}.
> {{Variant}}, {{BinaryVariant}} and {{BinaryVariantUtil}} stay unchanged.
> h3. Observable changes
> ||Expression||Before||After||
> |{{CAST(PARSE_JSON('"abc"') AS
> BYTES)}}|{{VariantTypeException}}|{{x'616263'}}|
> |{{CAST(PARSE_JSON('"abc"') AS
> BINARY(2))}}|{{VariantTypeException}}|{{x'6162'}}|
> |{{TRY_CAST(PARSE_JSON('"abc"') AS BYTES)}}|{{NULL}}|{{x'616263'}}|
> |{{CAST(PARSE_JSON('true') AS
> BYTES)}}|{{VariantTypeException}}|{{TableRuntimeException}} with the
> {{unsupportedKind}} message|
> The first row flips an existing expectation in {{CastRulesTest}}, which
> asserts that {{VARIANT_BUILDER.of("foo")}} fails a {{BYTES}} cast. The commit
> message calls this out. The table of cast targets in
> {{docs/content/docs/sql/reference/data-types.md}} gets a {{STRING}} to
> {{BINARY}} and {{VARBINARY}} entry.
> h3. Limits
> A stored {{STRING}} with an invalid UTF-8 payload loses its bytes.
> {{getString}} replaces malformed input with U+FFFD, so the cast returns {{EF
> BF BD}} in its place. {{CAST(v AS STRING)}} makes the same substitution
> today. Such a value can't come from the builders, which take a Java
> {{String}}, or from {{PARSE_JSON}}, which rejects invalid UTF-8. It can only
> come from bytes wrapped without validation, such as the binary encoding of
> the raw format from FLINK-41323. Validating string payloads at ingest, as
> FLINK-40318 proposes, would close this gap. A test pins the substitution.
> h3. Out of scope
> * A raw-bytes accessor for string values, and UTF-8 validation on the
> character-string cast. Ingest validation would make both unnecessary.
> * The {{BOOLEAN}} reader and a malformed variant. FLINK-41301 covers them.
> The {{DATE}} reader already checks the kind since FLINK-41281.
> h3. Tests
> * In {{FromVariantConverterTest}}, a non-ASCII string variant reaches
> {{BYTES}}, {{VARBINARY(3)}} and {{BINARY(8)}} as its UTF-8 bytes, truncated
> or padded like a stored {{BYTES}} value. A string payload {{C3 28}} comes out
> as {{EF BF BD 28}}.
> * In {{FromVariantConverterTest}}, a boolean, an integer and an object
> variant fail a binary cast with a {{TableRuntimeException}}.
> {{CastRulesTest}} can't show the exception type, because {{CastExecutor}}
> wraps any {{Throwable}} in a {{TableRuntimeException}}.
> * In {{CastRulesTest}}, the string case of the {{BYTES}} cast passes.
> * In {{CastFunctionITCase}}, {{CAST}} of a string variant to {{BYTES}}
> returns its bytes. {{CAST}} of a boolean one fails with the
> {{unsupportedKind}} message, and {{TRY_CAST}} returns {{NULL}}.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
