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/09/12 08:55:57 UTC

[GitHub] storm pull request #1681: STORM-1444 Support EXPLAIN statement in StormSQL

GitHub user HeartSaVioR opened a pull request:

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

    STORM-1444 Support EXPLAIN statement in StormSQL

    * when '--explain' is specified to topology name, activate explain mode
    * when explain mode is activated, it analyzes each query statement, and shows query plan
      * it doesn't submit topologies
    * Showing plan is provided by Calcite: it just uses that feature
    
    As I also described to the doc, query plan for below query 
    ```
    INSERT INTO LARGE_ORDERS SELECT ID, UNIT_PRICE * QUANTITY AS TOTAL FROM ORDERS 
    WHERE UNIT_PRICE * QUANTITY > 50
    ```
    
    is shown as: 
    
    ```
    LogicalTableModify(table=[[LARGE_ORDERS]], operation=[INSERT], updateColumnList=[[]], flattened=[true]), id = 8
      LogicalProject(ID=[$0], TOTAL=[*($1, $2)]), id = 7
        LogicalFilter(condition=[>(*($1, $2), 50)]), id = 6
          EnumerableTableScan(table=[[ORDERS]]), id = 5
    ```
    
    FYI, $0 is column index 0 (starting from 0), and so on. Expression is printed as prefix notation.
    Calcite just provides this functionality, though I'm not clear whether this is friendly (acceptable by end-user) or not.
    There's staled issue from Calcite ([CALCITE-155](https://issues.apache.org/jira/browse/CALCITE-155)) for displaying actual column names/aliases in explain plan.
    
    Please have a look and share your opinion about this.
    Thanks in advance!

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

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

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

    https://github.com/apache/storm/pull/1681.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 #1681
    
----

----


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

    https://github.com/apache/storm/pull/1681
  
    '!storm-core' fails due to the timeout of downloading artifact. I tested it manually.


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

    https://github.com/apache/storm/pull/1681
  
    FYI:
    Flink seems not support explain yet. 
    https://github.com/apache/flink/pull/2485 is proposed for adding this feature.
    
    As I described to JIRA issue, Spark supports explain and output seems to be more user friendly.
    (Note that Spark doesn't use Calcite.)
    
    ```
    scala> spark.sql("SELECT GRPID, COUNT(*) AS CNT, MAX(AGE) AS MAX_AGE, MIN(AGE) AS MIN_AGE, AVG(AGE) AS AVG_AGE, MAX(AGE) - MIN(AGE) AS DIFF FROM FOO WHERE ID > 2 GROUP BY GRPID").explain
    == Physical Plan ==
    *HashAggregate(keys=[GRPID#64], functions=[count(1), max(AGE#67), min(AGE#67), avg(cast(AGE#67 as bigint)), max(AGE#67), min(AGE#67)])
    +- Exchange hashpartitioning(GRPID#64, 200)
       +- *HashAggregate(keys=[GRPID#64], functions=[partial_count(1), partial_max(AGE#67), partial_min(AGE#67), partial_avg(cast(AGE#67 as bigint)), partial_max(AGE#67), partial_min(AGE#67)])
          +- *Project [grpid#64, age#67]
             +- *Filter (isnotnull(ID#63) && (ID#63 > 2))
                +- LocalTableScan [id#63, grpid#64, name#65, addr#66, age#67]
    ```


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

    https://github.com/apache/storm/pull/1681
  
    +1


---
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 #1681: STORM-1444 Support EXPLAIN statement 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/1681#discussion_r78543991
  
    --- Diff: external/sql/storm-sql-core/src/jvm/org/apache/storm/sql/StormSqlImpl.java ---
    @@ -140,6 +142,40 @@ public void submit(
         }
       }
     
    +  @Override
    +  public void explain(Iterable<String> statements) throws Exception {
    +    Map<String, ISqlTridentDataSource> dataSources = new HashMap<>();
    +    for (String sql : statements) {
    +      StormParser parser = new StormParser(sql);
    +      SqlNode node = parser.impl().parseSqlStmtEof();
    +
    +      System.out.println("===========================================================");
    +      System.out.println("query>");
    +      System.out.println(sql);
    +      System.out.println("-----------------------------------------------------------");
    +
    +      if (node instanceof SqlCreateTable) {
    +        handleCreateTableForTrident((SqlCreateTable) node, dataSources);
    +        System.out.println("No plan presented on DDL");
    +      } else if (node instanceof SqlCreateFunction) {
    +        handleCreateFunction((SqlCreateFunction) node);
    --- End diff --
    
    Yeah seems like it's missed spot. Thanks for finding.


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

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


---
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 #1681: STORM-1444 Support EXPLAIN statement 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/1681#discussion_r79392746
  
    --- Diff: external/sql/storm-sql-core/src/jvm/org/apache/storm/sql/StormSqlImpl.java ---
    @@ -140,6 +142,40 @@ public void submit(
         }
       }
     
    +  @Override
    +  public void explain(Iterable<String> statements) throws Exception {
    +    Map<String, ISqlTridentDataSource> dataSources = new HashMap<>();
    +    for (String sql : statements) {
    +      StormParser parser = new StormParser(sql);
    +      SqlNode node = parser.impl().parseSqlStmtEof();
    +
    +      System.out.println("===========================================================");
    +      System.out.println("query>");
    +      System.out.println(sql);
    +      System.out.println("-----------------------------------------------------------");
    +
    +      if (node instanceof SqlCreateTable) {
    +        handleCreateTableForTrident((SqlCreateTable) node, dataSources);
    +        System.out.println("No plan presented on DDL");
    +      } else if (node instanceof SqlCreateFunction) {
    +        handleCreateFunction((SqlCreateFunction) node);
    --- End diff --
    
    Addressed.


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

    https://github.com/apache/storm/pull/1681#discussion_r78530932
  
    --- Diff: external/sql/storm-sql-core/src/jvm/org/apache/storm/sql/StormSqlImpl.java ---
    @@ -140,6 +142,40 @@ public void submit(
         }
       }
     
    +  @Override
    +  public void explain(Iterable<String> statements) throws Exception {
    +    Map<String, ISqlTridentDataSource> dataSources = new HashMap<>();
    +    for (String sql : statements) {
    +      StormParser parser = new StormParser(sql);
    +      SqlNode node = parser.impl().parseSqlStmtEof();
    +
    +      System.out.println("===========================================================");
    +      System.out.println("query>");
    +      System.out.println(sql);
    +      System.out.println("-----------------------------------------------------------");
    +
    +      if (node instanceof SqlCreateTable) {
    +        handleCreateTableForTrident((SqlCreateTable) node, dataSources);
    +        System.out.println("No plan presented on DDL");
    +      } else if (node instanceof SqlCreateFunction) {
    +        handleCreateFunction((SqlCreateFunction) node);
    --- End diff --
    
    Do we need a tip here? `System.out.println("No plan presented on DDL");`


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

    https://github.com/apache/storm/pull/1681
  
    Btw, I'm ok with current explain output. Spark's is even better, maybe there is a chance to improve Calcite.


---
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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL

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

    https://github.com/apache/storm/pull/1681
  
    Nice Job. one minor comment. LGTM.


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