You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2016/08/18 07:57:05 UTC

[GitHub] storm pull request #1633: STORM-1434 Support the GROUP BY clause in StormSQL

GitHub user HeartSaVioR opened a pull request:

    https://github.com/apache/storm/pull/1633

    STORM-1434 Support the GROUP BY clause in StormSQL

    * Support GROUP BY for Trident
    * Implement basic functions for aggregation
    * Change the way of converting Calcite logical plan to Trident logical plan
    ** before: creating codes and compile them
    ** after: use Trident features, only creating code block if evaluation is needed
    *** Janino comes in to help evaluating code block in runtime
    
    This change can handle the below sql statements in runtime:
    
    ```
    CREATE EXTERNAL TABLE ORDERS (ID INT PRIMARY KEY, UNIT_PRICE INT, QUANTITY INT) LOCATION 'kafka://localhost:2181/brokers?topic=orders' TBLPROPERTIES '{"producer":{"bootstrap.servers":"localhost:9092","acks":"1","key.serializer":"org.apache.storm.kafka.IntSerializer","value.serializer":"org.apache.storm.kafka.ByteBufferSerializer"}}'
    CREATE EXTERNAL TABLE LARGE_ORDERS (ID INT PRIMARY KEY, TOTAL INT) LOCATION 'kafka://localhost:2181/brokers?topic=large_orders' TBLPROPERTIES '{"producer":{"bootstrap.servers":"localhost:9092","acks":"1","key.serializer":"org.apache.storm.kafka.IntSerializer","value.serializer":"org.apache.storm.kafka.ByteBufferSerializer"}}'
    INSERT INTO LARGE_ORDERS SELECT ID, UNIT_PRICE * QUANTITY AS TOTAL FROM ORDERS WHERE UNIT_PRICE * QUANTITY > 50
    ```
    
    ```
    CREATE EXTERNAL TABLE ORDERS (ID INT PRIMARY KEY, UNIT_PRICE INT, QUANTITY INT) LOCATION 'kafka://localhost:2181/brokers?topic=orders' TBLPROPERTIES '{"producer":{"bootstrap.servers":"localhost:9092","acks":"1","key.serializer":"org.apache.storm.kafka.IntSerializer","value.serializer":"org.apache.storm.kafka.ByteBufferSerializer"}}'
    CREATE EXTERNAL TABLE SUMMARY_ORDERS (ID INT PRIMARY KEY, UNIT_PRICE_AVG DOUBLE, SUM_OF_QUANTITY INT) LOCATION 'kafka://localhost:2181/brokers?topic=large_orders' TBLPROPERTIES '{"producer":{"bootstrap.servers":"localhost:9092","acks":"1","key.serializer":"org.apache.storm.kafka.IntSerializer","value.serializer":"org.apache.storm.kafka.ByteBufferSerializer"}}'
    INSERT INTO SUMMARY_ORDERS SELECT ID, AVG(UNIT_PRICE) AS UNIT_PRICE_AVG, SUM(QUANTITY) AS SUM_OF_QUANTITY FROM ORDERS GROUP BY ID
    ```
    
    This can be easily backported to 1.x branch since it only touches storm-sql module.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/HeartSaVioR/storm STORM-1454

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/1633.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 #1633
    
----
commit fc93fdad03c25967e7b00cc9c2289f46d71fb0a9
Author: Jungtaek Lim <ka...@gmail.com>
Date:   2016-08-18T07:32:46Z

    STORM-1434 Support the GROUP BY clause in StormSQL
    
    * Support GROUP BY for Trident
    * Implement basic functions for aggregation
    * Change the way of converting Calcite logical plan to Trident logical plan
    ** before: creating codes and compile them
    ** after: use Trident features, only creating code block if evaluation is needed
    *** Janino comes in to help evaluating code block in runtime

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1633: STORM-1434 Support the GROUP BY clause in StormSQL

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/1633
  
    Bad branch name. Will resubmit a new pull request with proper branch name soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1633: STORM-1434 Support the GROUP BY clause in StormSQL

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR closed the pull request at:

    https://github.com/apache/storm/pull/1633


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1633: STORM-1434 Support the GROUP BY clause in StormSQL

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/1633
  
    When reviewing, please share your idea if you find something to optimize further.
    I didn't play with Trident hardly so I'm not sure I'm following the best practice.
    
    I didn't touch UDF yet since coverage of STORM-1434 would be too broad if STORM-1434 contains UDF. Will file an issue to track and plan to address that.
    
    Also STORM-2016 should be addressed soon in order to run the topology.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1633: STORM-1434 Support the GROUP BY clause in StormSQL

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1633#discussion_r75264055
  
    --- Diff: external/sql/storm-sql-core/src/jvm/org/apache/storm/sql/compiler/ExprCompiler.java ---
    @@ -67,8 +68,9 @@ public ExprCompiler(PrintWriter pw, JavaTypeFactory typeFactory) {
       public String visitInputRef(RexInputRef rexInputRef) {
         String name = reserveName();
         String typeName = javaTypeName(rexInputRef);
    +    String boxedTypeName = boxedJavaTypeName(rexInputRef);
    --- End diff --
    
    Actually converting Object to primitive type is allowed for Java 7 or upper, but Janino throws error while compiling script so I just converted to boxed type.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1633: STORM-1434 Support the GROUP BY clause in StormSQL

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/1633
  
    Need to modify README to state that trident mode of storm-sql can support the "group by" with basic aggregation functions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---