twalthr commented on code in PR #29310:
URL: https://github.com/apache/flink/pull/29310#discussion_r4123022446
##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1593,6 +1593,12 @@ object as `{k1=v1, k2=v2}`, with each value rendered by
these same rules and a n
shown as `NULL`. A string is never quoted, at any depth. Use `JSON_STRING` for
the JSON form with
quoted strings. A variant that stores a JSON `null` casts to SQL `NULL`.
+Printed results, for example in the SQL client, render a `VARIANT` like a cast
to a character
Review Comment:
> like a cast to a character
is that true? I thought we are not using JSON for casting. Maybe I forgot a
previous decision.
##########
flink-core/src/main/java/org/apache/flink/types/variant/Variant.java:
##########
@@ -237,7 +240,12 @@ public interface Variant extends Serializable {
*/
List<String> getFieldNames() throws VariantTypeException;
- /** Parses the variant to json. */
+ /**
Review Comment:
We should add examples similar to the doc for STRICT and LENIENT mode, so
that people are aware of the concrete corner cases.
##########
flink-core/src/main/java/org/apache/flink/types/variant/Variant.java:
##########
@@ -237,7 +240,12 @@ public interface Variant extends Serializable {
*/
List<String> getFieldNames() throws VariantTypeException;
- /** Parses the variant to json. */
+ /**
+ * Returns the variant as valid JSON. Use {@code toString()} to debug a
variant without one.
+ *
+ * @throws VariantTypeException if the variant contains a value that
cannot be represented as
+ * valid JSON
Review Comment:
```suggestion
* valid JSON (e.g. NaN for double types)
```
##########
flink-core/src/main/java/org/apache/flink/types/variant/Variant.java:
##########
@@ -237,7 +240,12 @@ public interface Variant extends Serializable {
*/
List<String> getFieldNames() throws VariantTypeException;
- /** Parses the variant to json. */
+ /**
+ * Returns the variant as valid JSON. Use {@code toString()} to debug a
variant without one.
+ *
+ * @throws VariantTypeException if the variant contains a value that
cannot be represented as
+ * valid JSON
Review Comment:
also add a toString with a JavaDoc to this interface that covers UNKNOWN and
other special cases.
##########
flink-core/src/main/java/org/apache/flink/types/variant/JsonVariantFormatter.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.types.variant;
+
+import org.apache.flink.annotation.Internal;
+
+import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonFactory;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonGenerator;
+
+import java.io.CharArrayWriter;
+import java.io.IOException;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.ZoneOffset;
+import java.util.Base64;
+
+import static
org.apache.flink.types.variant.BinaryVariantUtil.TIMESTAMP_FORMATTER;
+import static
org.apache.flink.types.variant.BinaryVariantUtil.TIMESTAMP_LTZ_FORMATTER;
+import static org.apache.flink.types.variant.BinaryVariantUtil.TIME_FORMATTER;
+import static org.apache.flink.types.variant.BinaryVariantUtil.getMetadataKey;
+import static org.apache.flink.types.variant.BinaryVariantUtil.handleArray;
+import static org.apache.flink.types.variant.BinaryVariantUtil.handleObject;
+import static org.apache.flink.types.variant.BinaryVariantUtil.microsToInstant;
+import static org.apache.flink.types.variant.BinaryVariantUtil.nanosToInstant;
+import static org.apache.flink.types.variant.BinaryVariantUtil.readUnsigned;
+import static org.apache.flink.types.variant.BinaryVariantUtil.unexpectedType;
+
+/**
+ * Renders a {@link BinaryVariant} as JSON.
+ *
+ * <p>{@link #STRICT} backs {@link Variant#toJson()} and fails on NaN,
infinity, and undecodable
+ * nodes. {@link #LENIENT} backs {@link BinaryVariant#toString()} and never
fails: it writes NaN as
+ * {@code "NaN"}, a type this version does not know as {@code "<UNKNOWN>"},
and other undecodable
+ * data as {@code "<INVALID>"}.
+ */
+@Internal
+public final class JsonVariantFormatter {
+
+ public static final JsonVariantFormatter STRICT = new
JsonVariantFormatter(false);
+
+ public static final JsonVariantFormatter LENIENT = new
JsonVariantFormatter(true);
+
+ private static final JsonFactory JSON_FACTORY = new JsonFactory();
+
+ private final boolean lenient;
+
+ private JsonVariantFormatter(final boolean lenient) {
+ this.lenient = lenient;
+ }
+
+ public String format(final BinaryVariant variant) {
+ final StringBuilder sb = new StringBuilder();
+ append(variant.rawValue(), variant.getMetadata(), variant.getPos(),
sb);
+ return sb.toString();
+ }
+
+ private void append(
+ final byte[] value, final byte[] metadata, final int pos, final
StringBuilder sb) {
+ final int start = sb.length();
+ try {
+ appendNode(value, metadata, pos, sb);
+ } catch (VariantTypeException e) {
+ if (!lenient) {
+ throw e;
+ }
+ // Drop the node's partial output, such as a dangling key.
+ sb.setLength(start);
+ appendQuoted(sb, BinaryVariantUtil.undecodableNode(value, pos));
+ }
+ }
+
+ private void appendNode(byte[] value, byte[] metadata, int pos,
StringBuilder sb) {
+ switch (BinaryVariantUtil.getType(value, pos)) {
+ case OBJECT:
+ handleObject(
+ value,
+ pos,
+ (size, idSize, offsetSize, idStart, offsetStart,
dataStart) -> {
+ sb.append('{');
+ for (int i = 0; i < size; ++i) {
+ int id = readUnsigned(value, idStart + idSize
* i, idSize);
+ int offset =
+ readUnsigned(
+ value, offsetStart +
offsetSize * i, offsetSize);
+ int elementPos = dataStart + offset;
+ if (i != 0) {
+ sb.append(',');
+ }
+ sb.append(escapeJson(getMetadataKey(metadata,
id)));
+ sb.append(':');
+ append(value, metadata, elementPos, sb);
+ }
+ sb.append('}');
+ return null;
+ });
+ break;
+ case ARRAY:
+ handleArray(
+ value,
+ pos,
+ (size, offsetSize, offsetStart, dataStart) -> {
+ sb.append('[');
+ for (int i = 0; i < size; ++i) {
+ int offset =
+ readUnsigned(
+ value, offsetStart +
offsetSize * i, offsetSize);
+ int elementPos = dataStart + offset;
+ if (i != 0) {
+ sb.append(',');
+ }
+ append(value, metadata, elementPos, sb);
+ }
+ sb.append(']');
+ return null;
+ });
+ break;
+ case NULL:
+ sb.append("null");
+ break;
+ case BOOLEAN:
+ sb.append(BinaryVariantUtil.getBoolean(value, pos));
+ break;
+ case TINYINT:
+ case SMALLINT:
+ case INT:
+ case BIGINT:
+ // Reads the 1, 2, 4 or 8 bytes the header declares and widens
them to a long.
+ sb.append(BinaryVariantUtil.getLong(value, pos));
+ break;
+ case STRING:
+ sb.append(escapeJson(BinaryVariantUtil.getString(value, pos)));
+ break;
+ case DOUBLE:
+ {
+ final double d = BinaryVariantUtil.getDouble(value, pos);
+ if (Double.isFinite(d)) {
+ sb.append(d);
+ } else {
+ appendNonFinite(sb, Double.toString(d), d);
+ }
+ break;
+ }
+ case DECIMAL:
+ sb.append(BinaryVariantUtil.getDecimal(value,
pos).toPlainString());
+ break;
+ case DATE:
+ appendQuoted(
+ sb,
+ LocalDate.ofEpochDay((int)
BinaryVariantUtil.getLong(value, pos))
+ .toString());
+ break;
+ case TIMESTAMP_LTZ:
+ appendQuoted(
+ sb,
+ TIMESTAMP_LTZ_FORMATTER.format(
+
microsToInstant(BinaryVariantUtil.getLong(value, pos))
+ .atZone(ZoneOffset.UTC)));
+ break;
+ case TIMESTAMP:
+ appendQuoted(
+ sb,
+ TIMESTAMP_FORMATTER.format(
+
microsToInstant(BinaryVariantUtil.getLong(value, pos))
+ .atZone(ZoneOffset.UTC)));
+ break;
+ case TIME:
+ appendQuoted(
+ sb,
+ TIME_FORMATTER.format(
+ LocalTime.ofNanoOfDay(
+ BinaryVariantUtil.getLong(value, pos)
* 1000)));
+ break;
+ case TIMESTAMP_LTZ_NS:
+ appendQuoted(
+ sb,
+ TIMESTAMP_LTZ_FORMATTER.format(
+
nanosToInstant(BinaryVariantUtil.getLong(value, pos))
+ .atZone(ZoneOffset.UTC)));
+ break;
+ case TIMESTAMP_NS:
+ appendQuoted(
+ sb,
+ TIMESTAMP_FORMATTER.format(
+
nanosToInstant(BinaryVariantUtil.getLong(value, pos))
+ .atZone(ZoneOffset.UTC)));
+ break;
+ case FLOAT:
+ {
+ final float f = BinaryVariantUtil.getFloat(value, pos);
+ if (Float.isFinite(f)) {
+ sb.append(f);
+ } else {
+ appendNonFinite(sb, Float.toString(f), f);
+ }
+ break;
+ }
+ case BYTES:
+ appendQuoted(
+ sb,
+ Base64.getEncoder()
+
.encodeToString(BinaryVariantUtil.getBinary(value, pos)));
+ break;
+ case UUID:
+ appendQuoted(sb, BinaryVariantUtil.getUuid(value,
pos).toString());
+ break;
+ default:
+ throw unexpectedType(BinaryVariantUtil.getType(value, pos));
+ }
+ }
+
+ /** JSON has no literal for NaN or infinity. */
+ private void appendNonFinite(final StringBuilder sb, final String text,
final double number) {
+ if (!lenient) {
+ throw new VariantTypeException(
+ String.format("Non-finite value %s cannot be serialized to
JSON.", number));
Review Comment:
```suggestion
String.format("Non-finite value '%s' cannot be
serialized to JSON.", number));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
