[jira] [Commented] (BEAM-3238) [SQL] Add builder to BeamRecordSqlType

2017-11-28 Thread ASF GitHub Bot (JIRA)

[ 
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 Map SQL_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

2017-11-28 Thread ASF GitHub Bot (JIRA)

[ 
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

2017-11-28 Thread ASF GitHub Bot (JIRA)

[ 
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

2017-11-24 Thread ASF GitHub Bot (JIRA)

[ 
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

2017-11-22 Thread ASF GitHub Bot (JIRA)

[ 
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 Kedin 
Date:   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)