[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-11 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1729
  
Pushed to master branch also.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-11 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1729
  
Thanks @arunmahadevan for the fix.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-11 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1729
  
Spend some time to look into it but it looks like breaking changes are 
needed.
Interpreter expects static input table before initializing Interpreter, 
which is valid for most of case, but Storm SQL standalone mode works like 
streaming, emitting input tuples and receive the outputs.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1729
  
I guess `Interpreter` seems to be candidate for replacement of standalone 
more on storm-sql.


https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/interpreter/Interpreter.java

Unit tests are here.


https://github.com/apache/calcite/blob/master/core/src/test/java/org/apache/calcite/test/InterpreterTest.java

It still need to register tables and functions to root schema but storm-sql 
is same.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1729
  
I guess Calcite itself can provide interpreter for handling query. If then 
standalone users can just move to Calcite interpreter, and standalone mode 
could be removed. I'm planning to find it from 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread harshach
Github user harshach commented on the issue:

https://github.com/apache/storm/pull/1729
  
+1. Isn't standalone mode still helpful for devs instead of just moving to 
abstract API.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1729
  
+1 for replacing code generation with an abstract API as it is hard to 
maintain. But that can be a different task and it is not related to this PR.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread arunmahadevan
Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1729
  
@HeartSaVioR you are right, its very tedious to maintain. Right now its 
kind of a bridge to translate sql to java so that sql abstractions can be 
executed in storm core. IMO we should get out of standalone mode in future and 
add support in storm-sql to directly translate to storm-core topologies via the 
streams api proposed in STORM-1961 (https://github.com/apache/storm/pull/1693).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1729
  
Other than that I'm +1. IMHO, standalone mode also needs to have some 
abstraction or introduce something to not creating source code by manipulate 
string directly.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1729
  
Found storm-jms version issue while checking Travis CI failing. Will submit 
a patch shortly.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm issue #1729: [STORM-2144] Fix Storm-sql group-by behavior in standalon...

2016-10-10 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1729
  
+1 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---