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/06/18 18:34:52 UTC

[GitHub] [beam] apilloud commented on a change in pull request #12027: [BEAM-9179] Refactor Beam ZetaSQL type translation code

apilloud commented on a change in pull request #12027:
URL: https://github.com/apache/beam/pull/12027#discussion_r442424214



##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSqlCalciteTranslationUtils.java
##########
@@ -43,55 +40,38 @@
 import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rex.RexBuilder;
 import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableList;
-import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableMap;
 
-/** Utility to convert types from Calcite Schema types. */
+/**
+ * Utility methods for ZetaSQL <=> Calcite translation.
+ *
+ * <p>Unsupported ZetaSQL types: INT32, UINT32, UINT64, FLOAT, ENUM, PROTO, GEOGRAPHY
+ * TODO[BEAM-10238]: support ZetaSQL types: TIME, DATETIME, NUMERIC
+ */
 @Internal
-public class TypeUtils {
-
-  private static final ImmutableMap<SqlTypeName, Type> CALCITE_TO_ZETA_SIMPLE_TYPES =
-      ImmutableMap.<SqlTypeName, Type>builder()
-          .put(SqlTypeName.BIGINT, TypeFactory.createSimpleType(TYPE_INT64))
-          .put(SqlTypeName.INTEGER, TypeFactory.createSimpleType(TYPE_INT32))
-          .put(SqlTypeName.VARCHAR, TypeFactory.createSimpleType(TYPE_STRING))
-          .put(SqlTypeName.BOOLEAN, TypeFactory.createSimpleType(TYPE_BOOL))
-          .put(SqlTypeName.FLOAT, TypeFactory.createSimpleType(TYPE_FLOAT))
-          .put(SqlTypeName.DOUBLE, TypeFactory.createSimpleType(TYPE_DOUBLE))
-          .put(SqlTypeName.VARBINARY, TypeFactory.createSimpleType(TYPE_BYTES))
-          .put(SqlTypeName.TIMESTAMP, TypeFactory.createSimpleType(TYPE_TIMESTAMP))
-          .put(SqlTypeName.DATE, TypeFactory.createSimpleType(TYPE_DATE))
-          .put(SqlTypeName.TIME, TypeFactory.createSimpleType(TYPE_TIME))
-          .build();
-
-  private static final ImmutableMap<TypeKind, Function<RexBuilder, RelDataType>>
-      ZETA_TO_CALCITE_SIMPLE_TYPES =
-          ImmutableMap.<TypeKind, Function<RexBuilder, RelDataType>>builder()
-              .put(TYPE_NUMERIC, relDataTypeFactory(SqlTypeName.DECIMAL))
-              .put(TYPE_INT32, relDataTypeFactory(SqlTypeName.INTEGER))
-              .put(TYPE_INT64, relDataTypeFactory(SqlTypeName.BIGINT))
-              .put(TYPE_FLOAT, relDataTypeFactory(SqlTypeName.FLOAT))
-              .put(TYPE_DOUBLE, relDataTypeFactory(SqlTypeName.DOUBLE))
-              .put(TYPE_STRING, relDataTypeFactory(SqlTypeName.VARCHAR))
-              .put(TYPE_BOOL, relDataTypeFactory(SqlTypeName.BOOLEAN))
-              .put(TYPE_BYTES, relDataTypeFactory(SqlTypeName.VARBINARY))
-              .put(TYPE_DATE, relDataTypeFactory(SqlTypeName.DATE))
-              .put(TYPE_TIME, relDataTypeFactory(SqlTypeName.TIME))
-              // TODO: handle timestamp with time zone.
-              .put(TYPE_TIMESTAMP, relDataTypeFactory(SqlTypeName.TIMESTAMP))
-              .build();
-
-  /** Returns a type matching the corresponding Calcite type. */
-  static Type toZetaType(RelDataType calciteType) {
-
-    if (CALCITE_TO_ZETA_SIMPLE_TYPES.containsKey(calciteType.getSqlTypeName())) {
-      return CALCITE_TO_ZETA_SIMPLE_TYPES.get(calciteType.getSqlTypeName());
-    }
+public class ZetaSqlCalciteTranslationUtils {
 
+  // Type conversion: Calcite => ZetaSQL
+  static Type toZetaType(RelDataType calciteType) {
     switch (calciteType.getSqlTypeName()) {
+      case BIGINT:
+        return TypeFactory.createSimpleType(TYPE_INT64);
+      case DOUBLE:
+        return TypeFactory.createSimpleType(TYPE_DOUBLE);
+      case BOOLEAN:
+        return TypeFactory.createSimpleType(TYPE_BOOL);
+      case VARCHAR:
+        return TypeFactory.createSimpleType(TYPE_STRING);
+      case VARBINARY:
+        return TypeFactory.createSimpleType(TYPE_BYTES);
+      case DATE:
+        return TypeFactory.createSimpleType(TYPE_DATE);
+      case TIME:

Review comment:
       I believe #10158 dropped the need for MAP. You might verify it can be removed with @TheNeuralBit 

##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSqlCalciteTranslationUtils.java
##########
@@ -134,7 +138,7 @@ public static RelDataType toArrayRelDataType(
     for (int i = 0; i < fields.size(); i++) {
       String name = fields.get(i).getName();
       if ("".equals(name)) {
-        name = "$col" + String.valueOf(i);

Review comment:
       Thanks for cleaning up after my C developer instinct to call itoa everywhere.




----------------------------------------------------------------
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