You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/17 04:19:51 UTC
[GitHub] [beam] robinyqiu commented on a change in pull request #12292: [BEAM-8057] Support ZetaSQL DOUBLE +INF, -INF and NAN
robinyqiu commented on a change in pull request #12292:
URL: https://github.com/apache/beam/pull/12292#discussion_r456207986
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/ExpressionConverter.java
##########
@@ -786,15 +786,39 @@ private RexNode convertSimpleValueToRexNode(TypeKind kind, Value value) {
ZetaSqlCalciteTranslationUtils.toSimpleRelDataType(kind, rexBuilder()));
break;
case TYPE_DOUBLE:
+ // Cannot simply call makeApproxLiteral() for ZetaSQL DOUBLE type because positive infinity,
+ // negative infinity and Nan cannot be directly converted to BigDecimal. So we create three
+ // wrapper functions here for these three cases such that we can later recognize it and
+ // customize its unparsing in BeamBigQuerySqlDialect.
double val = value.getDoubleValue();
- if (Double.isInfinite(val) || Double.isNaN(val)) {
- throw new UnsupportedOperationException("Does not support Infinite or NaN literals.");
+ if (Double.isInfinite(val) && val > 0) {
+ ret =
+ rexBuilder()
+ .makeCall(
+ SqlOperators.createOtherKindSqlFunction(
+ BeamBigQuerySqlDialect.DOUBLE_POSITIVE_INF_FUNCTION,
+ ZetaSqlCalciteTranslationUtils.toCalciteTypeName(kind)));
+ } else if (Double.isInfinite(val) && val < 0) {
+ ret =
+ rexBuilder()
+ .makeCall(
+ SqlOperators.createOtherKindSqlFunction(
+ BeamBigQuerySqlDialect.DOUBLE_NEGATIVE_INF_FUNCTION,
+ ZetaSqlCalciteTranslationUtils.toCalciteTypeName(kind)));
+ } else if (Double.isNaN(val)) {
+ ret =
+ rexBuilder()
+ .makeCall(
+ SqlOperators.createOtherKindSqlFunction(
+ BeamBigQuerySqlDialect.DOUBLE_NAN_FUNCTION,
+ ZetaSqlCalciteTranslationUtils.toCalciteTypeName(kind)));
+ } else {
+ ret =
+ rexBuilder()
+ .makeApproxLiteral(
+ new BigDecimal(val),
+ ZetaSqlCalciteTranslationUtils.toSimpleRelDataType(kind, rexBuilder()));
Review comment:
I could create 2 local parameters: `String wrapperFun` (null for the last case) and `RelDataType returnType` to simplify this big if-else block.
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/ExpressionConverter.java
##########
@@ -786,15 +786,39 @@ private RexNode convertSimpleValueToRexNode(TypeKind kind, Value value) {
ZetaSqlCalciteTranslationUtils.toSimpleRelDataType(kind, rexBuilder()));
break;
case TYPE_DOUBLE:
+ // Cannot simply call makeApproxLiteral() for ZetaSQL DOUBLE type because positive infinity,
+ // negative infinity and Nan cannot be directly converted to BigDecimal. So we create three
+ // wrapper functions here for these three cases such that we can later recognize it and
+ // customize its unparsing in BeamBigQuerySqlDialect.
double val = value.getDoubleValue();
- if (Double.isInfinite(val) || Double.isNaN(val)) {
- throw new UnsupportedOperationException("Does not support Infinite or NaN literals.");
+ if (Double.isInfinite(val) && val > 0) {
Review comment:
How about just check `val == Double.POSITIVE_INFINITY` here? (similar below)
##########
File path: sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BeamBigQuerySqlDialect.java
##########
@@ -157,8 +160,20 @@ public void unparseCall(
break;
case OTHER_FUNCTION:
String funName = call.getOperator().getName();
- if (NUMERIC_LITERAL_FUNCTION.equals(funName)) {
- // self-designed function dealing with the unparsing of ZetaSQL numeric literal
+ if (DOUBLE_POSITIVE_INF_FUNCTION.equals(funName)) {
Review comment:
If you create a map `DOUBLE_LITERAL_FUNCTIONS`, you can merge the 3 branches here and the 3 functions below. (Similar to what I did with `EXTRACT_FUNCTIONS`)
##########
File path: sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BeamBigQuerySqlDialect.java
##########
@@ -239,6 +254,18 @@ private void unparseTrim(SqlWriter writer, SqlCall call, int leftPrec, int right
writer.endFunCall(trimFrame);
}
+ private void unparseDoublePositiveINFWrapperFunction(SqlWriter writer) {
+ writer.literal("CAST('+inf' AS FLOAT64)");
Review comment:
Maybe comment here: "There is no direct ZetaSQL literal representation of NaN or infinity"?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org