[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-12 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 @arunmahadevan Yes ideally we really want to have it. I just said that ExprCompiler seems not having a value to keep and maintain. Agreed with you. Thanks for reviewing! --- If your project is

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-12 Thread arunmahadevan
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1714 +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 fe

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-12 Thread arunmahadevan
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1714 @HeartSaVioR I suggested to keep an option to plug our code generator. If you think calcites code generator (linq4j) addresses all the current cases then lets go with that. We can revisit havin

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-12 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 @arunmahadevan I'm not sure we can have our own expression compiler which supports features competitive to Calcite. It reminds me why this change is necessary. We have been leaving e

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-12 Thread arunmahadevan
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1714 @HeartSaVioR with this change we are going to be heavily dependent on the calcite's code generator to translate expressions correct?. Just thinking if we should have an option to make the code

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-11 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Calcite 1.10.0 RC1 vote passed and artifact is available to Maven. Applied HeartSaVioR@b2bcf9b with changing Calcite version from 1.10.0-SNAPSHOT to 1.10.0. Also rebased and squashed the c

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-08 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Just found and fixed bugs regarding aggregate calls and timezone issue in current_date. Now aggregate also needs new Trident map API - STORM-2072 (#1663) to reduce the steps. I'll address

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-07 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 FYI: The vote for 1.10.0 RC1 is just started so we can expect Calcite 1.10.0 to next week. Submitted patches for recently found bugs to Calcite: CALCITE-1418 and CALCITE-1419. Above

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Addressed documenting covered feature from other PR: #1725 --- 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

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Added full of scalar function tests which Calcite provides. Calcite 1.9.0 has some bugs so not all functions are working well, but it can be fixed from Calcite side. (I left comments to test

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Filed [STORM-2135](https://issues.apache.org/jira/browse/STORM-2135) and [STORM-2136](https://issues.apache.org/jira/browse/STORM-2136). I'd like to get some helps on these issues, but don't mind

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 @arunmahadevan Regarding Calcite release, Calcite community is waiting for one issue to get resolved before starting RC. I guess RC will be available in this week or so. I'll watch Calc

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-10-05 Thread arunmahadevan
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1714 @HeartSaVioR +1 on this change. If the minor version of the calcite with your fixes can get released soon, it makes sense to wait for that so we can keep the current semantics for querying nes

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Another head up: my patch got merged to Calcite master, and Calcite dev@ is discussing about releasing new minor version soon. --- If your project is set up for it, you can reply to this email a

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 Just a head up: my patch for Calcite (https://github.com/apache/calcite/pull/283) passes the review and in progress of merging. (handling some comment fixes.) So I'm expecting that we ca

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 I'm trying to address our concern of nested map / array since Calcite community (Julian) says it sounds like a bug. Addressing it from CALCITE-1386 : https://github.com/apache/calcite/pu

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-26 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 @arunmahadevan Other than that I addressed your review comments. Thanks for the thoughtful review. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-26 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 @arunmahadevan Let me summary regarding nested map/array support. * accessing nested element in nested map/array works without modifying Calcite. * it should be defined as

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-26 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 I don't want to get blocked by non-SQL standard support unless we have clear use case or clear way to implement it. If my understanding is right, both Spark and Flink seems not support this as we

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-26 Thread arunmahadevan
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1714 Can you also document the list of functions that are now available since we are directly using calicite to generate the java code? Does it support all the standard SQL functions? --- If your

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-25 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 There's an another issue: when to initialize DataContext. Calcite utilizes the variables in DataContext. and there're datetime related variables in DataContext. The thing is, SQL