MartijnVisser commented on code in PR #29506:
URL: https://github.com/apache/flink/pull/29506#discussion_r4307433863
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/JsonValueCallGen.scala:
##########
@@ -66,43 +67,81 @@ class JsonValueCallGen extends CallGenerator {
val parsed = JsonParseReuse.parseSharedInput(ctx, operands)
- generateCallWithStmtIfArgsNotNull(ctx, returnType, operands,
resultNullable = true) {
- argTerms =>
- {
- val emptyBehavior = getBehavior(operands, SqlJsonEmptyOrError.EMPTY)
- val errorBehavior = getBehavior(operands, SqlJsonEmptyOrError.ERROR)
+ val generated =
+ generateCallWithStmtIfArgsNotNull(ctx, returnType, operands,
resultNullable = true) {
+ argTerms =>
+ {
+ val emptyBehavior = getBehavior(operands,
SqlJsonEmptyOrError.EMPTY)
+ val errorBehavior = getBehavior(operands,
SqlJsonEmptyOrError.ERROR)
- val terms = Seq(
- parsed.resultTerm,
- s"${argTerms(1)}.toString()",
- qualifyEnum(emptyBehavior._1),
- emptyBehavior._2,
- qualifyEnum(errorBehavior._1),
- errorBehavior._2
- )
+ val terms = Seq(
+ parsed.resultTerm,
+ s"${argTerms(1)}.toString()",
+ qualifyEnum(emptyBehavior._1),
+ emptyBehavior._2,
+ qualifyEnum(errorBehavior._1),
+ errorBehavior._2
+ )
- val rawResultTerm = CodeGenUtils.newName(ctx, "rawResult")
- val call = s"""
- |Object $rawResultTerm =
- |
${qualifyMethod(BuiltInMethods.JSON_VALUE)}(${terms
- .mkString(", ")});
+ val rawResultTerm = CodeGenUtils.newName(ctx, "rawResult")
+ val baseCall = s"""
+ |Object $rawResultTerm =
+ |
${qualifyMethod(BuiltInMethods.JSON_VALUE)}(${terms
+ .mkString(", ")});
""".stripMargin
- val convertedResult = returnType.getTypeRoot match {
- case LogicalTypeRoot.VARCHAR =>
-
s"$BINARY_STRING.fromString(java.lang.String.valueOf($rawResultTerm))"
- case LogicalTypeRoot.BOOLEAN => s"(java.lang.Boolean)
$rawResultTerm"
- case LogicalTypeRoot.INTEGER => s"(java.lang.Integer)
$rawResultTerm"
- case LogicalTypeRoot.DOUBLE => s"((java.math.BigDecimal)
$rawResultTerm).doubleValue()"
- case _ =>
- throw new CodeGenException(
- s"Unsupported type '$returnType' "
- + "for RETURNING in JSON_VALUE().")
+ returnType.getTypeRoot match {
+ case LogicalTypeRoot.VARCHAR =>
+ val result =
+ s"($rawResultTerm == null) ? null : " +
+
s"($BINARY_STRING.fromString(java.lang.String.valueOf($rawResultTerm)))"
+ (baseCall, result)
+ case typeRoot if
!SqlJsonUtils.isSupportedJsonReturningType(typeRoot) =>
+ throw new CodeGenException(
+ s"Unsupported type '$returnType' for RETURNING in
JSON_VALUE().")
+ case _ =>
+ val jsonUtils =
+ classOf[SqlJsonUtils].getName
+ val typeRootEnum =
+
s"${classOf[LogicalTypeRoot].getName}.${returnType.getTypeRoot.name()}"
+ val (precisionStr, scaleStr) = returnType.getTypeRoot match {
+ case LogicalTypeRoot.DECIMAL =>
+ val dt = returnType.asInstanceOf[DecimalType]
+ (dt.getPrecision.toString, dt.getScale.toString)
+ case _ => ("0", "0")
+ }
+ val errorBehaviorEnum =
+
qualifyEnum(errorBehavior._1.asInstanceOf[JsonValueOnEmptyOrError])
+ val defaultExpr =
+ if (errorBehavior._2 != null) errorBehavior._2 else "null"
+
+ val boxedType = CodeGenUtils.boxedTypeTermForType(returnType)
+ val convertedTerm = CodeGenUtils.newName(ctx, "converted")
+ val call = baseCall +
+ s"""
+ |$boxedType $convertedTerm = ($boxedType)
$jsonUtils.convertJsonScalar(
+ | $rawResultTerm, $typeRootEnum,
+ | $precisionStr, $scaleStr,
+ | $errorBehaviorEnum, $defaultExpr);
+ """.stripMargin
+ (call, s"$convertedTerm")
+ }
}
+ }
- val result = s"($rawResultTerm == null) ? null : ($convertedResult)"
- (call, result)
- }
+ if (isPrimitiveNullable(returnType)) {
Review Comment:
The boxed result bug is also on 1.20 and 2.x (`=` on two `RETURNING INT`
values of 1000 is FALSE). Can this be its own commit and ticket for the
backports?
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/data/binary/BinaryStringDataUtil.java:
##########
@@ -155,15 +155,15 @@ public static BinaryStringData[]
splitByWholeSeparatorPreserveAllTokens(
}
/** Parse a {@link StringData} to boolean. */
- public static boolean toBoolean(BinaryStringData str) throws
TableException {
+ public static boolean toBoolean(BinaryStringData str) {
BinaryStringData lowerCase = str.toLowerCase();
if (TRUE_STRINGS.contains(lowerCase)) {
return true;
}
if (FALSE_STRINGS.contains(lowerCase)) {
return false;
}
- throw new TableException("Cannot parse '" + str + "' as BOOLEAN.");
+ throw new TableRuntimeException("Cannot cast '" + str + "' as
BOOLEAN.");
Review Comment:
I think this changes the exception and message of every `CAST(... AS
BOOLEAN)` on a string, not only the JSON path. Is that intended?
##########
docs/data/sql_functions.yml:
##########
@@ -1086,8 +1086,18 @@ json:
* `VARCHAR` / `STRING`
* `BOOLEAN`
+ * `TINYINT`
+ * `SMALLINT`
* `INTEGER`
+ * `BIGINT`
+ * `FLOAT`
* `DOUBLE`
+ * `DECIMAL`
+
+ JSON string values are coerced to the requested numeric or boolean type
(for example `"42"`
+ to a number, or `"true"`/`"t"`/`"yes"`/`"y"`/`"1"` to `TRUE`), matching
`CAST` semantics.
Review Comment:
`"1e2"` gives 100 for INTEGER and `" true "` gives TRUE, while `CAST` fails
on both, and `"NaN"` is the other way around. I'd drop "matching `CAST`
semantics" or align them.
--
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]
