[GitHub] vlsi commented on a change in pull request #1042: Avoid use of new RuntimeException(e) in tests

2019-02-14 Thread GitBox
vlsi commented on a change in pull request #1042: Avoid use of new RuntimeException(e) in tests URL: https://github.com/apache/calcite/pull/1042#discussion_r256724918 ## File path: core/src/test/java/org/apache/calcite/test/CalciteAssert.java ## @@ -315,6 +305,7 @@

new Adapter Contribution

2019-02-14 Thread Hanan Yehudai
Hi all, we are developing a calcite based adapter, are there any pre-requirements to contribute it as part of calcite ? is there another Apache project for adapter ( like Baheer for Flink/ Spark sinks )?

Re: Global LOOKAHEAD of the SQL parser

2019-02-14 Thread Hongze Zhang
I think it's not something unacceptable to require parser developers to know the usage of LOOKAHEAD hints of JavaCC, for keeping the parser efficient and ambiguous free. Firstly, it's not really that difficult to learn. By setting the global LOOKAHEAD to 1, JavaCC will perform ambiguity

[GitHub] zhztheplayer commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
zhztheplayer commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#discussion_r256784061 ## File path:

[GitHub] kgyrtkirk opened a new pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
kgyrtkirk opened a new pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044 …checks Earlier expressions like ((x IS TRUE) IS TRUE) were left as is, the new behaviour recognizes if the IS

[GitHub] vlsi commented on a change in pull request #1042: Avoid use of new RuntimeException(e) in tests

2019-02-14 Thread GitBox
vlsi commented on a change in pull request #1042: Avoid use of new RuntimeException(e) in tests URL: https://github.com/apache/calcite/pull/1042#discussion_r256849041 ## File path: core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java ## @@ -8153,7

[GitHub] vlsi commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
vlsi commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#discussion_r256836978 ## File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java ## @@

[GitHub] kgyrtkirk commented on issue #980: [CALCITE-2736] Update the ReduceExpressionsRule to better expose options

2019-02-14 Thread GitBox
kgyrtkirk commented on issue #980: [CALCITE-2736] Update the ReduceExpressionsRule to better expose options URL: https://github.com/apache/calcite/pull/980#issuecomment-463647710 I think having something to help configure `ReduceExpressionRule` would be good; `ReductionOptions` could be a

[GitHub] vlsi commented on a change in pull request #1042: Avoid use of new RuntimeException(e) in tests

2019-02-14 Thread GitBox
vlsi commented on a change in pull request #1042: Avoid use of new RuntimeException(e) in tests URL: https://github.com/apache/calcite/pull/1042#discussion_r256851757 ## File path: core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java ## @@ -8153,7

[GitHub] vlsi commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
vlsi commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#discussion_r256890036 ## File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java ## @@

Re: Failed to parse a PostgreSQL query using the Babel conformance

2019-02-14 Thread Muhammad Gelbana
@Stamatis, I very appreciate you taking the time to comment on the issues I opened basd on this thread. I'm currently going through Babel's Parser.jj file and JavaCC documentations trying to understand what I need to do and where. Considering you're probably more acquainted than I am. I'll gladly

[GitHub] zhztheplayer commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
zhztheplayer commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#discussion_r256895426 ## File path:

[GitHub] zhztheplayer commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
zhztheplayer commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#discussion_r256903413 ## File path:

Re: new Adapter Contribution

2019-02-14 Thread Michael Mior
There is no separate project which collects Calcite adapters. You will find a few projects around with different adapters that haven't been accepted into Calcite for various reasons (the author doesn't desire to, it's unlikely to be broadly useful, etc.) In general, I'd say we're always interested

[GitHub] jszeluga commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
jszeluga commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#discussion_r256862657 ## File path:

[GitHub] vlsi commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
vlsi commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#discussion_r256865065 ## File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java ## @@

[GitHub] kgyrtkirk commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
kgyrtkirk commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#discussion_r256860106 ## File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java ##

[GitHub] kgyrtkirk commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
kgyrtkirk commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#issuecomment-463654705 I kinda went a little defensive after the jira comments :) If you mean; rewrite `x IS FALSE` to `not x` in

[GitHub] kgyrtkirk commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
kgyrtkirk commented on a change in pull request #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#discussion_r256872580 ## File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java ##

[GitHub] kgyrtkirk opened a new pull request #1045: [CALCITE-2840] Simplification should use more specific UnknownAs mode…

2019-02-14 Thread GitBox
kgyrtkirk opened a new pull request #1045: [CALCITE-2840] Simplification should use more specific UnknownAs mode… URL: https://github.com/apache/calcite/pull/1045 …s during simplification Earlier nested AND/OR expressions might not were able to exploit the specific simplification

[GitHub] vlsi commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
vlsi commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#issuecomment-463635300 Hey @kgyrtkirk , I see you have a nice PR here. What do you think on `IS_NOT_TRUE` / `IS_FALSE`? Would

[GitHub] kgyrtkirk commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
kgyrtkirk commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#issuecomment-463748212 sure, I'll include a brief explanation about why these work. I think we can only add these 4 right now.

Re: new RuntimeException(Throwable) vs SneakyThrows

2019-02-14 Thread Julian Hyde
Do you propose to ban ’throw new RuntimeException(e)’ in all code or just in tests? The alternative, ’throw rethrow(e)' is only available in test code (because rethrow lives in TestUtil). What should people do in non-test code? Continue to ’throw new RuntimeException’? Use guava’s ’throw

Re: new Adapter Contribution

2019-02-14 Thread Julian Hyde
I suggest that, as a bare minimum, you log a JIRA case for the adapter that you want / are building. In that case you can describe the problem you are solving, and other people can find you and collaborate. You can start developing the adapter in a Calcite branch. Make it a separate maven

CalciteRemoteDriverTest vs avatica.MetaImpl vs avatica.server.Main thread safety

2019-02-14 Thread Vladimir Sitnikov
Hi, CalciteRemoteDriverTest seems to be very unstable at https://builds.apache.org/job/Calcite-Master/ I guess the problem there is the test is using a single CalciteMetaImpl object to handle all the requests. Apparently CalciteMetaImpl is using no synchronization and it uses simple HashMaps

[GitHub] jszeluga commented on issue #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
jszeluga commented on issue #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#issuecomment-463755028 What do I have to fix to resolve the conflict?

[GitHub] jszeluga commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
jszeluga commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#discussion_r256930157 ## File path:

Re: new RuntimeException(Throwable) vs SneakyThrows

2019-02-14 Thread Stamatis Zampetakis
Thanks for starting this discussion Vladimir. Although sneaky throws pattern is a rather hacky way of propagating a checked exception it does improve its readability. I like it. Till now I never noticed how verbose was a checked exception wrapped in a runtime exception. I agree that the pattern

[GitHub] vlsi commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
vlsi commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#issuecomment-463701536 >If you mean; rewrite `x IS FALSE` to `not x` in case of `UnknownAs.False` ; I think that will work, I'll include

[GitHub] jszeluga commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi…

2019-02-14 Thread GitBox
jszeluga commented on a change in pull request #1014: [CALCITE-2804] fix casting to timestamps because of malformed dimensi… URL: https://github.com/apache/calcite/pull/1014#discussion_r256930157 ## File path:

Re: Global LOOKAHEAD of the SQL parser

2019-02-14 Thread Julian Hyde
Thanks for the clarification. You have convinced me that modifying the parser will not be too onerous. I was pleased to see that only one change was required to the “server” parser, for instance. I saw that you overrode the tester in BabelParserTest. I makes sense, but it creates an exposure:

[GitHub] kgyrtkirk commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE …

2019-02-14 Thread GitBox
kgyrtkirk commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant IS TRUE/IS NOT FALSE … URL: https://github.com/apache/calcite/pull/1044#issuecomment-463818974 >> By the way, why don't you consider IS_TRUE+UnknownAs.TRUE and other Unknown.As.TRUE cases? > we are

Re: new RuntimeException(Throwable) vs SneakyThrows

2019-02-14 Thread Vladimir Sitnikov
Julian> Do you propose to ban ’throw new RuntimeException(e)’ in all code or just in tests? I'm 100.42% sure we should ban new RuntimeException(e) and all the other cases like "new Throwable(e)" in test code. pull/1042 touches test code only, so it is more-or-less simple to reason about. That is

Re: Global LOOKAHEAD of the SQL parser

2019-02-14 Thread Hongze Zhang
> Is there a way to have that tester check error messages if the test is > defined in BabelParserTest? (Perhaps you could inspect the stack and see > whether “BabelParserTest.test” appears in the top few entries.) Thanks Julian! And this makes sense. I'll give it a try in the later

Re: CalciteRemoteDriverTest vs avatica.MetaImpl vs avatica.server.Main thread safety

2019-02-14 Thread Josh Elser
The Meta implementation inside of the Avatica server is a singleton, so yes the implementation must be threadsafe as the server will dispatch multiple user request threads to it. The Meta implementation is largely (by virtue of the wire API) stateless. I think the expectation should be that

[jira] [Created] (CALCITE-2848) Simplifying a case statement's first branch should disregard its safety

2019-02-14 Thread Zoltan Haindrich (JIRA)
Zoltan Haindrich created CALCITE-2848: - Summary: Simplifying a case statement's first branch should disregard its safety Key: CALCITE-2848 URL: https://issues.apache.org/jira/browse/CALCITE-2848

[GitHub] kgyrtkirk opened a new pull request #1046: [CALCITE-2848] Simplifying a case statement's first branch should ign…

2019-02-14 Thread GitBox
kgyrtkirk opened a new pull request #1046: [CALCITE-2848] Simplifying a case statement's first branch should ign… URL: https://github.com/apache/calcite/pull/1046 …ore its safety Earlier when a CASE expression had any condition/value which contained probably dangerous instructions,

[GitHub] kgyrtkirk opened a new pull request #1047: [CALCITE-2841] Simplification: push negation into Case expression

2019-02-14 Thread GitBox
kgyrtkirk opened a new pull request #1047: [CALCITE-2841] Simplification: push negation into Case expression URL: https://github.com/apache/calcite/pull/1047 Earlier NOT was not pushed into CASE expressions. Also ensure that recursion under NOT happens.