[jira] [Commented] (BEAM-3238) [SQL] Add builder to BeamRecordSqlType
[ https://issues.apache.org/jira/browse/BEAM-3238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270113#comment-16270113 ] ASF GitHub Bot commented on BEAM-3238: -- xumingming closed pull request #4168: [BEAM-3238][SQL] Add BeamRecordSqlTypeBuilder URL: https://github.com/apache/beam/pull/4168 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/BeamRecordSqlType.java b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/BeamRecordSqlType.java index 982494ad2e5..a6b23b6310b 100644 --- a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/BeamRecordSqlType.java +++ b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/BeamRecordSqlType.java @@ -17,13 +17,14 @@ */ package org.apache.beam.sdk.extensions.sql; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import java.math.BigDecimal; import java.sql.Types; import java.util.ArrayList; import java.util.Collections; import java.util.Date; import java.util.GregorianCalendar; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.beam.sdk.coders.BigDecimalCoder; @@ -50,26 +51,39 @@ * */ public class BeamRecordSqlType extends BeamRecordType { - private static final MapSQL_TYPE_TO_JAVA_CLASS = new HashMap<>(); - static { -SQL_TYPE_TO_JAVA_CLASS.put(Types.TINYINT, Byte.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.SMALLINT, Short.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.INTEGER, Integer.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.BIGINT, Long.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.FLOAT, Float.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.DOUBLE, Double.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.DECIMAL, BigDecimal.class); - -SQL_TYPE_TO_JAVA_CLASS.put(Types.BOOLEAN, Boolean.class); - -SQL_TYPE_TO_JAVA_CLASS.put(Types.CHAR, String.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.VARCHAR, String.class); - -SQL_TYPE_TO_JAVA_CLASS.put(Types.TIME, GregorianCalendar.class); - -SQL_TYPE_TO_JAVA_CLASS.put(Types.DATE, Date.class); -SQL_TYPE_TO_JAVA_CLASS.put(Types.TIMESTAMP, Date.class); - } + private static final Map JAVA_CLASSES = ImmutableMap + . builder() + .put(Types.TINYINT, Byte.class) + .put(Types.SMALLINT, Short.class) + .put(Types.INTEGER, Integer.class) + .put(Types.BIGINT, Long.class) + .put(Types.FLOAT, Float.class) + .put(Types.DOUBLE, Double.class) + .put(Types.DECIMAL, BigDecimal.class) + .put(Types.BOOLEAN, Boolean.class) + .put(Types.CHAR, String.class) + .put(Types.VARCHAR, String.class) + .put(Types.TIME, GregorianCalendar.class) + .put(Types.DATE, Date.class) + .put(Types.TIMESTAMP, Date.class) + .build(); + + private static final Map CODERS = ImmutableMap + . builder() + .put(Types.TINYINT, ByteCoder.of()) + .put(Types.SMALLINT, ShortCoder.of()) + .put(Types.INTEGER, BigEndianIntegerCoder.of()) + .put(Types.BIGINT, BigEndianLongCoder.of()) + .put(Types.FLOAT, FloatCoder.of()) + .put(Types.DOUBLE, DoubleCoder.of()) + .put(Types.DECIMAL, BigDecimalCoder.of()) + .put(Types.BOOLEAN, BooleanCoder.of()) + .put(Types.CHAR, StringUtf8Coder.of()) + .put(Types.VARCHAR, StringUtf8Coder.of()) + .put(Types.TIME, TimeCoder.of()) + .put(Types.DATE, DateCoder.of()) + .put(Types.TIMESTAMP, DateCoder.of()) + .build(); public List fieldTypes; @@ -84,54 +98,24 @@ private BeamRecordSqlType(List fieldsName, List fieldTypes } public static BeamRecordSqlType create(List fieldNames, - List fieldTypes) { + List fieldTypes) { if (fieldNames.size() != fieldTypes.size()) { throw new IllegalStateException("the sizes of 'dataType' and 'fieldTypes' must match."); } + List fieldCoders = new ArrayList<>(fieldTypes.size()); + for (int idx = 0; idx < fieldTypes.size(); ++idx) { - switch (fieldTypes.get(idx)) { - case Types.INTEGER: -fieldCoders.add(BigEndianIntegerCoder.of()); -break; - case Types.SMALLINT: -fieldCoders.add(ShortCoder.of()); -break; - case Types.TINYINT: -fieldCoders.add(ByteCoder.of()); -break; - case Types.DOUBLE: -fieldCoders.add(DoubleCoder.of()); -break; - case Types.FLOAT: -fieldCoders.add(FloatCoder.of()); -break; - case Types.DECIMAL: -
[jira] [Commented] (BEAM-3238) [SQL] Add builder to BeamRecordSqlType
[ https://issues.apache.org/jira/browse/BEAM-3238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269639#comment-16269639 ] ASF GitHub Bot commented on BEAM-3238: -- akedin commented on issue #4168: [BEAM-3238][SQL] Add BeamRecordSqlTypeBuilder URL: https://github.com/apache/beam/pull/4168#issuecomment-347693930 retest this please. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [SQL] Add builder to BeamRecordSqlType > -- > > Key: BEAM-3238 > URL: https://issues.apache.org/jira/browse/BEAM-3238 > Project: Beam > Issue Type: Improvement > Components: dsl-sql >Reporter: Anton Kedin >Assignee: Anton Kedin > > Currently it's hard to match field names with types when constructing a > BeamRecordSqlType, like > [here|https://github.com/apache/beam/blob/39e66e953b0f8e16435acb038cad364acf2b3a57/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/integrationtest/BeamSqlBuiltinFunctionsIntegrationTestBase.java#L64-L71]: > {code:java} > BeamRecordSqlType type = BeamRecordSqlType.create( > Arrays.asList("ts", "c_tinyint", "c_smallint", > "c_integer", "c_bigint", "c_float", "c_double", "c_decimal", > "c_tinyint_max", "c_smallint_max", "c_integer_max", "c_bigint_max"), > Arrays.asList(Types.DATE, Types.TINYINT, Types.SMALLINT, > Types.INTEGER, Types.BIGINT, Types.FLOAT, Types.DOUBLE, Types.DECIMAL, > Types.TINYINT, Types.SMALLINT, Types.INTEGER, Types.BIGINT) > ); > {code} > It would be much more readable to have a builder, along these lines: > {code:java} > BeamRecordSqlType.builder() > .withField("f_int", Types.INTEGER) > .withStringField("f_str") > .build(); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (BEAM-3238) [SQL] Add builder to BeamRecordSqlType
[ https://issues.apache.org/jira/browse/BEAM-3238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16268367#comment-16268367 ] ASF GitHub Bot commented on BEAM-3238: -- xumingming commented on issue #4168: [BEAM-3238][SQL] Add BeamRecordSqlTypeBuilder URL: https://github.com/apache/beam/pull/4168#issuecomment-347448647 retest this please. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [SQL] Add builder to BeamRecordSqlType > -- > > Key: BEAM-3238 > URL: https://issues.apache.org/jira/browse/BEAM-3238 > Project: Beam > Issue Type: Improvement > Components: dsl-sql >Reporter: Anton Kedin >Assignee: Anton Kedin > > Currently it's hard to match field names with types when constructing a > BeamRecordSqlType, like > [here|https://github.com/apache/beam/blob/39e66e953b0f8e16435acb038cad364acf2b3a57/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/integrationtest/BeamSqlBuiltinFunctionsIntegrationTestBase.java#L64-L71]: > {code:java} > BeamRecordSqlType type = BeamRecordSqlType.create( > Arrays.asList("ts", "c_tinyint", "c_smallint", > "c_integer", "c_bigint", "c_float", "c_double", "c_decimal", > "c_tinyint_max", "c_smallint_max", "c_integer_max", "c_bigint_max"), > Arrays.asList(Types.DATE, Types.TINYINT, Types.SMALLINT, > Types.INTEGER, Types.BIGINT, Types.FLOAT, Types.DOUBLE, Types.DECIMAL, > Types.TINYINT, Types.SMALLINT, Types.INTEGER, Types.BIGINT) > ); > {code} > It would be much more readable to have a builder, along these lines: > {code:java} > BeamRecordSqlType.builder() > .withField("f_int", Types.INTEGER) > .withStringField("f_str") > .build(); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (BEAM-3238) [SQL] Add builder to BeamRecordSqlType
[ https://issues.apache.org/jira/browse/BEAM-3238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16265289#comment-16265289 ] ASF GitHub Bot commented on BEAM-3238: -- xumingming commented on issue #4168: [BEAM-3238][SQL] Add BeamRecordSqlTypeBuilder URL: https://github.com/apache/beam/pull/4168#issuecomment-346831152 I like this idea! One minor comment: Can we put the `BeamRecordSqlTypeBuilder` inside `BeamRecordSqlType`? it will keep the surface api of `org.apache.beam.sdk.extensions.sql` cleaner. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [SQL] Add builder to BeamRecordSqlType > -- > > Key: BEAM-3238 > URL: https://issues.apache.org/jira/browse/BEAM-3238 > Project: Beam > Issue Type: Improvement > Components: dsl-sql >Reporter: Anton Kedin >Assignee: Anton Kedin > > Currently it's hard to match field names with types when constructing a > BeamRecordSqlType, like > [here|https://github.com/apache/beam/blob/39e66e953b0f8e16435acb038cad364acf2b3a57/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/integrationtest/BeamSqlBuiltinFunctionsIntegrationTestBase.java#L64-L71]: > {code:java} > BeamRecordSqlType type = BeamRecordSqlType.create( > Arrays.asList("ts", "c_tinyint", "c_smallint", > "c_integer", "c_bigint", "c_float", "c_double", "c_decimal", > "c_tinyint_max", "c_smallint_max", "c_integer_max", "c_bigint_max"), > Arrays.asList(Types.DATE, Types.TINYINT, Types.SMALLINT, > Types.INTEGER, Types.BIGINT, Types.FLOAT, Types.DOUBLE, Types.DECIMAL, > Types.TINYINT, Types.SMALLINT, Types.INTEGER, Types.BIGINT) > ); > {code} > It would be much more readable to have a builder, along these lines: > {code:java} > BeamRecordSqlType.builder() > .withField("f_int", Types.INTEGER) > .withStringField("f_str") > .build(); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (BEAM-3238) [SQL] Add builder to BeamRecordSqlType
[ https://issues.apache.org/jira/browse/BEAM-3238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263746#comment-16263746 ] ASF GitHub Bot commented on BEAM-3238: -- GitHub user akedin opened a pull request: https://github.com/apache/beam/pull/4168 [BEAM-3238][SQL] Add BeamRecordSqlTypeBuilder Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/projects/BEAM/issues/) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- You can merge this pull request into a Git repository by running: $ git pull https://github.com/akedin/beam sql-record-type-builder Alternatively you can review and apply these changes as the patch at: https://github.com/apache/beam/pull/4168.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4168 commit 77769aa46dd9a972526f805f6c476c4039121b29 Author: Anton KedinDate: 2017-11-23T00:04:05Z [BEAM-3238][SQL] Move Coders to map in BeamRecordSqlType Improve readability. All coders are supposed to be thread safe and currently are backed by the static instances. commit 96c2a67f38c3da501f37797a8f75c23d5baf2ca8 Author: Anton Kedin Date: 2017-11-23T03:17:21Z [BEAM-3238][SQL] Add BeamRecordSqlTypeBuilder To improve readability of creating BeamRecordSqlTypes. > [SQL] Add builder to BeamRecordSqlType > -- > > Key: BEAM-3238 > URL: https://issues.apache.org/jira/browse/BEAM-3238 > Project: Beam > Issue Type: Improvement > Components: dsl-sql >Reporter: Anton Kedin >Assignee: Anton Kedin > > Currently it's hard to match field names with types when constructing a > BeamRecordSqlType, like > [here|https://github.com/apache/beam/blob/39e66e953b0f8e16435acb038cad364acf2b3a57/sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/integrationtest/BeamSqlBuiltinFunctionsIntegrationTestBase.java#L64-L71]: > {code:java} > BeamRecordSqlType type = BeamRecordSqlType.create( > Arrays.asList("ts", "c_tinyint", "c_smallint", > "c_integer", "c_bigint", "c_float", "c_double", "c_decimal", > "c_tinyint_max", "c_smallint_max", "c_integer_max", "c_bigint_max"), > Arrays.asList(Types.DATE, Types.TINYINT, Types.SMALLINT, > Types.INTEGER, Types.BIGINT, Types.FLOAT, Types.DOUBLE, Types.DECIMAL, > Types.TINYINT, Types.SMALLINT, Types.INTEGER, Types.BIGINT) > ); > {code} > It would be much more readable to have a builder, along these lines: > {code:java} > BeamRecordSqlType.builder() > .withField("f_int", Types.INTEGER) > .withStringField("f_str") > .build(); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)