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:
[email protected]
