[jira] [Commented] (CALCITE-4787) Evaluate use of Immutables instead of ImmutableBeans
[
https://issues.apache.org/jira/browse/CALCITE-4787?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17418763#comment-17418763
]
Laurent Goujon commented on CALCITE-4787:
-
{quote}
My last concern is compatibility. If a dependent project is using a different
version of Immutables, what will be the impact?
{quote}
To have use immutables before, there's zero issue. Source code is emitted by
the Java compiler, and has zero dependency to immutables code (I believe that
all immutables annotations are source only too).
> Evaluate use of Immutables instead of ImmutableBeans
>
>
> Key: CALCITE-4787
> URL: https://issues.apache.org/jira/browse/CALCITE-4787
> Project: Calcite
> Issue Type: Improvement
>Reporter: Jacques Nadeau
>Assignee: Jacques Nadeau
>Priority: Major
> Labels: pull-request-available
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> In the creation of CALCITE-3328, [Immutables|https://immutables.github.io/]
> was discussed as an alternative to a custom implementation. This ticket is to
> evaluate the impact to the codebase of changing. Ideally, introduction of
> immutables would both add flexibility and reduce the amount of code
> associated with these classes.
> Immutables works via annotation processor which means that it is should be
> relatively seamless to build systems and IDEs.
> The switch would also make it easier to work with these objects types in the
> context of aot compilation tools like GraalVM.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4224) Add an method for RelNode to output its tree digest like RelOptUtil.toString
[
https://issues.apache.org/jira/browse/CALCITE-4224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17193075#comment-17193075
]
Laurent Goujon commented on CALCITE-4224:
-
I'm not sure I see how much more convenient it is to switch one method with
another one. The only issue I experienced was that {{RelOptUtil}} is not always
in the source context and I need to use the fully qualify class name instead of
just the simple name.
On the other hand, having a public overridable method to go over the whole tree
seems too broad (does the method needs to access internal state? is it okay for
subclasses to override?). I guess I'm -0 on that, but would be okay for it to
exist as a static method in {{RelNode}}
> Add an method for RelNode to output its tree digest like RelOptUtil.toString
>
>
> Key: CALCITE-4224
> URL: https://issues.apache.org/jira/browse/CALCITE-4224
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Jiatao Tao
>Assignee: Jiatao Tao
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
[
https://issues.apache.org/jira/browse/CALCITE-4025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17181450#comment-17181450
]
Laurent Goujon commented on CALCITE-4025:
-
Thinking more about it (and before changing the issue's title), I wonder if
there's not a second issue at hand which is the case where both the alias and
the column have the exact same case, and it would be ambiguous to resolve but
currently it does not fail?
> DelegatingScope#fullyQualify may exit too soon when resolving table alias
> -
>
> Key: CALCITE-4025
> URL: https://issues.apache.org/jira/browse/CALCITE-4025
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
> there's a look going over all the possible sub prefixes and trying to resolve
> them, until one succeed. But if not, the same lookup is done using a liberal
> name matcher. If that one resolves, then the method exit early with an
> exception message instead of going over the whole loop or trying for a fully
> qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
[
https://issues.apache.org/jira/browse/CALCITE-4025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17181443#comment-17181443
]
Laurent Goujon commented on CALCITE-4025:
-
Sure. Does the test case look valid at least?
> DelegatingScope#fullyQualify may exit too soon when resolving table alias
> -
>
> Key: CALCITE-4025
> URL: https://issues.apache.org/jira/browse/CALCITE-4025
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
> there's a look going over all the possible sub prefixes and trying to resolve
> them, until one succeed. But if not, the same lookup is done using a liberal
> name matcher. If that one resolves, then the method exit early with an
> exception message instead of going over the whole loop or trying for a fully
> qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4179) org.apache.calcite.jdbc package polluting core Calcite planning packages
[
https://issues.apache.org/jira/browse/CALCITE-4179?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17181251#comment-17181251
]
Laurent Goujon commented on CALCITE-4179:
-
Likely. I went over open JIRAs to find similar issues but didn't see that one,
so thanks for pointing it to me.
One question I have is regarding to backward compatibility because breaking
those dependencies might require changes to public interfaces, and although we
could try to maintain those while using {{@Deprecated}}, it could also make
harder to enforce the policy for future changes.
> org.apache.calcite.jdbc package polluting core Calcite planning packages
>
>
> Key: CALCITE-4179
> URL: https://issues.apache.org/jira/browse/CALCITE-4179
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Priority: Major
>
> Several classes under {{org.apache.calcite.jdbc}} package are used by
> planning classes. As those JDBC classes are some form of implementation, it
> means the Calcite API has some kind of coupling, causing extra challenges for
> alternate implementations.
> One of the central classes of {{org.apache.calcite.jdbc}} is
> {{CalciteSchema}} which represents the catalog space. This class is designed
> to load the entire catalog and hold it in memory, which is a problem for
> those having catalogs too large for this to be practical. This class is
> sometimes used instead of the more generic {{Schema}} and {{SchemaPlus}}
> classes.
> Another similar class is {{CalcitePrepare}}: the class is for example used by
> {{org.apache.calcite.prepare.Prepare}} instead of the dependency being the
> other way around.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (CALCITE-4179) org.apache.calcite.jdbc package polluting core Calcite planning packages
Laurent Goujon created CALCITE-4179:
---
Summary: org.apache.calcite.jdbc package polluting core Calcite
planning packages
Key: CALCITE-4179
URL: https://issues.apache.org/jira/browse/CALCITE-4179
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Several classes under {{org.apache.calcite.jdbc}} package are used by planning
classes. As those JDBC classes are some form of implementation, it means the
Calcite API has some kind of coupling, causing extra challenges for alternate
implementations.
One of the central classes of {{org.apache.calcite.jdbc}} is {{CalciteSchema}}
which represents the catalog space. This class is designed to load the entire
catalog and hold it in memory, which is a problem for those having catalogs too
large for this to be practical. This class is sometimes used instead of the
more generic {{Schema}} and {{SchemaPlus}} classes.
Another similar class is {{CalcitePrepare}}: the class is for example used by
{{org.apache.calcite.prepare.Prepare}} instead of the dependency being the
other way around.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Resolved] (CALCITE-4092) NPE using WITH clause without a corresponding SELECT FROM
[
https://issues.apache.org/jira/browse/CALCITE-4092?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-4092.
-
Fix Version/s: 1.24.0
Resolution: Fixed
Merged as commit
[eb55b5f|https://github.com/apache/calcite/commit/eb55b5f93a27a5b68280e25abf37cb8cda112ad7]
> NPE using WITH clause without a corresponding SELECT FROM
> -
>
> Key: CALCITE-4092
> URL: https://issues.apache.org/jira/browse/CALCITE-4092
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: James Kim
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.24.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> Running the below query:
>
> {code:java}
> final String sql = "with emp2 as (select max(empno) as empno from emp where
> empno > 10)\n"
> + "select drill.* from emp drill where drill.empno < emp2.empno";
> {code}
> We hit NPE
>
> {code:java}
> while converting `DRILL`.`EMPNO` < `EMP2`.`EMPNO`java.lang.RuntimeException:
> while converting `DRILL`.`EMPNO` < `EMP2`.`EMPNO` at
> org.apache.calcite.sql2rel.ReflectiveConvertletTable.lambda$registerNodeTypeMethod$0(ReflectiveConvertletTable.java:86)
> at
> org.apache.calcite.sql2rel.SqlNodeToRexConverterImpl.convertCall(SqlNodeToRexConverterImpl.java:63)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:5006)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4302)
> at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:139) at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4869)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertWhere(SqlToRelConverter.java:1017)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:662)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:640)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3385)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:566)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertWith(SqlToRelConverter.java:4220)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3399)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:566)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:631)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:749)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:3938)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.ok(SqlToRelConverterTest.java:3929)
> at
> org.apache.calcite.test.SqlToRelConverterTest.testWithNotSelected(SqlToRelConverterTest.java:889)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498) at
> org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
> at
> org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125)
> at
> org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:139)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:131)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:81)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:104)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:62)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:43)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:35)
> at
>
[jira] [Updated] (CALCITE-4092) NPE using WITH clause without a corresponding SELECT FROM
[
https://issues.apache.org/jira/browse/CALCITE-4092?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-4092:
Component/s: core
> NPE using WITH clause without a corresponding SELECT FROM
> -
>
> Key: CALCITE-4092
> URL: https://issues.apache.org/jira/browse/CALCITE-4092
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: James Kim
>Priority: Major
>
> Running the below query:
>
> {code:java}
> final String sql = "with emp2 as (select max(empno) as empno from emp where
> empno > 10)\n"
> + "select drill.* from emp drill where drill.empno < emp2.empno";
> {code}
> We hit NPE
>
> {code:java}
> while converting `DRILL`.`EMPNO` < `EMP2`.`EMPNO`java.lang.RuntimeException:
> while converting `DRILL`.`EMPNO` < `EMP2`.`EMPNO` at
> org.apache.calcite.sql2rel.ReflectiveConvertletTable.lambda$registerNodeTypeMethod$0(ReflectiveConvertletTable.java:86)
> at
> org.apache.calcite.sql2rel.SqlNodeToRexConverterImpl.convertCall(SqlNodeToRexConverterImpl.java:63)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:5006)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4302)
> at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:139) at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4869)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertWhere(SqlToRelConverter.java:1017)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:662)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:640)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3385)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:566)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertWith(SqlToRelConverter.java:4220)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3399)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:566)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:631)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:749)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:3938)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.ok(SqlToRelConverterTest.java:3929)
> at
> org.apache.calcite.test.SqlToRelConverterTest.testWithNotSelected(SqlToRelConverterTest.java:889)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498) at
> org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
> at
> org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125)
> at
> org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:139)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:131)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:81)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:104)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:62)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:43)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:35)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:202)
[jira] [Comment Edited] (CALCITE-3786) Add Digest interface to enable efficient hashCode(equals) for RexNode and RelNode
[
https://issues.apache.org/jira/browse/CALCITE-3786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17137805#comment-17137805
]
Laurent Goujon edited comment on CALCITE-3786 at 6/17/20, 1:00 AM:
---
{quote}
Laurent Goujon It seems that you have some promotion ideas for the Digest code,
feel free to do that, i tried to answer each message you left in the PR, hope
it helps. But i would not revert the PR for no other strong reason, sorry.
{quote}
The thing is that you left no time to answer back (sorry, I'm not working over
the weekend), or did you send a courtesy check on either the pull request and
the jira issue to see if people got their questions answered/concerns
alleviated. We usually try to reach a consensus on issues and code changes, but
it doesn't seem to be the case here. And it's a bit disappointed you are not
ready to revert temporarily your change unless there are strong reason whereas
(ignoring my comments/concerns on github) the lack of concrete evaluation
regarding memory usage and performance impact seems quite a strong reason to do
so...
was (Author: laurentgo):
{quote}
Laurent Goujon It seems that you have some promotion ideas for the Digest code,
feel free to do that, i tried to answer each message you left in the PR, hope
it helps. But i would not revert the PR for no other strong reason, sorry.
{quote}
The thing is that you left no time to answer back (sorry, I'm not working over
the weekend), or did you send a courtesy check on either the pull request and
the jira issue to see people got their questions answers/concerns alleviated.
We usually try to reach a consensus on issues and code changes, but it doesn't
seem to be the case here. And it's a bit disappointed you are not ready to
revert temporarily your change unless there are strong reason whereas (ignoring
my comments/concerns on github) the lack of concrete evaluation regarding
memory usage and performance impact seems quite a strong reason to do so...
> Add Digest interface to enable efficient hashCode(equals) for RexNode and
> RelNode
> -
>
> Key: CALCITE-3786
> URL: https://issues.apache.org/jira/browse/CALCITE-3786
> Project: Calcite
> Issue Type: New Feature
> Components: core
>Affects Versions: 1.21.0
>Reporter: Vladimir Sitnikov
>Assignee: Danny Chen
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 5h 20m
> Remaining Estimate: 0h
>
> Current digests for RexNode, RelNode, RelType, and similar cases use String
> concatenation.
> It is easy to implement, however, it has drawbacks:
> 1) String objects cannot be reused. For instance, RexCall has operands,
> however, the digest is duplicated. It causes extra memory use and extra CPU
> for string copying
> 2) There's no way to have multiple #toString() methods. RelType might need
> multiple digests: "including field names", "excluding field names".
> A suggested resolution might be behind the lines of
> {code:java}
> class Digest { // immutable
> final int hashCode; // speedup hashCode and equals
> final Object[] contents; // The values are either other Digest objects or
> Strings
> String toString(); // e.g. for debugging purposes
> int compareTo(Digest); // e.g. for debugging purposes.
> }
> {code}
> Note how fields in Kotlin are aligned much better, and it makes it easier to
> read:
> {code:java}
> class Digest { // immutable
> val hashCode: Int // speedup hashCode and equals
> val contents: Array // The values are either other Digest objects or
> Strings
> fun toString(): String // e.g. for debugging purposes
> fun compareTo(other: Digest): Int // e.g. for debugging purposes.
> }
> {code}
> Then the digest for RexCall could be the bits relevant to RexCall itself +
> digests of the operands (which can be reused as is)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3786) Add Digest interface to enable efficient hashCode(equals) for RexNode and RelNode
[
https://issues.apache.org/jira/browse/CALCITE-3786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17137805#comment-17137805
]
Laurent Goujon commented on CALCITE-3786:
-
{quote}
Laurent Goujon It seems that you have some promotion ideas for the Digest code,
feel free to do that, i tried to answer each message you left in the PR, hope
it helps. But i would not revert the PR for no other strong reason, sorry.
{quote}
The thing is that you left no time to answer back (sorry, I'm not working over
the weekend), or did you send a courtesy check on either the pull request and
the jira issue to see people got their questions answers/concerns alleviated.
We usually try to reach a consensus on issues and code changes, but it doesn't
seem to be the case here. And it's a bit disappointed you are not ready to
revert temporarily your change unless there are strong reason whereas (ignoring
my comments/concerns on github) the lack of concrete evaluation regarding
memory usage and performance impact seems quite a strong reason to do so...
> Add Digest interface to enable efficient hashCode(equals) for RexNode and
> RelNode
> -
>
> Key: CALCITE-3786
> URL: https://issues.apache.org/jira/browse/CALCITE-3786
> Project: Calcite
> Issue Type: New Feature
> Components: core
>Affects Versions: 1.21.0
>Reporter: Vladimir Sitnikov
>Assignee: Danny Chen
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 5h 20m
> Remaining Estimate: 0h
>
> Current digests for RexNode, RelNode, RelType, and similar cases use String
> concatenation.
> It is easy to implement, however, it has drawbacks:
> 1) String objects cannot be reused. For instance, RexCall has operands,
> however, the digest is duplicated. It causes extra memory use and extra CPU
> for string copying
> 2) There's no way to have multiple #toString() methods. RelType might need
> multiple digests: "including field names", "excluding field names".
> A suggested resolution might be behind the lines of
> {code:java}
> class Digest { // immutable
> final int hashCode; // speedup hashCode and equals
> final Object[] contents; // The values are either other Digest objects or
> Strings
> String toString(); // e.g. for debugging purposes
> int compareTo(Digest); // e.g. for debugging purposes.
> }
> {code}
> Note how fields in Kotlin are aligned much better, and it makes it easier to
> read:
> {code:java}
> class Digest { // immutable
> val hashCode: Int // speedup hashCode and equals
> val contents: Array // The values are either other Digest objects or
> Strings
> fun toString(): String // e.g. for debugging purposes
> fun compareTo(other: Digest): Int // e.g. for debugging purposes.
> }
> {code}
> Then the digest for RexCall could be the bits relevant to RexCall itself +
> digests of the operands (which can be reused as is)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3786) Add Digest interface to enable efficient hashCode(equals) for RexNode and RelNode
[
https://issues.apache.org/jira/browse/CALCITE-3786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17136282#comment-17136282
]
Laurent Goujon commented on CALCITE-3786:
-
I'm not sure I understand why the change has been merged already: there has
been questions/concerns on both JIRA and Github about the change. Is it
possible to revert it and address those?
Personally, considering the impact, I would like to see a more concrete
benchmark on the memory impact. I have for example some reservation about the
use of a list of pair of string/objects in digest for things which are most
likely fields of the {{RelNode}}.
And there's also the digest String field, initialized lazily (but not always?),
but which is not sensible to memory pressure for example: could we for example
let this string be gc'ed if there's not enough memory at the time even it means
recomputing it?
> Add Digest interface to enable efficient hashCode(equals) for RexNode and
> RelNode
> -
>
> Key: CALCITE-3786
> URL: https://issues.apache.org/jira/browse/CALCITE-3786
> Project: Calcite
> Issue Type: New Feature
> Components: core
>Affects Versions: 1.21.0
>Reporter: Vladimir Sitnikov
>Assignee: Danny Chen
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 4h
> Remaining Estimate: 0h
>
> Current digests for RexNode, RelNode, RelType, and similar cases use String
> concatenation.
> It is easy to implement, however, it has drawbacks:
> 1) String objects cannot be reused. For instance, RexCall has operands,
> however, the digest is duplicated. It causes extra memory use and extra CPU
> for string copying
> 2) There's no way to have multiple #toString() methods. RelType might need
> multiple digests: "including field names", "excluding field names".
> A suggested resolution might be behind the lines of
> {code:java}
> class Digest { // immutable
> final int hashCode; // speedup hashCode and equals
> final Object[] contents; // The values are either other Digest objects or
> Strings
> String toString(); // e.g. for debugging purposes
> int compareTo(Digest); // e.g. for debugging purposes.
> }
> {code}
> Note how fields in Kotlin are aligned much better, and it makes it easier to
> read:
> {code:java}
> class Digest { // immutable
> val hashCode: Int // speedup hashCode and equals
> val contents: Array // The values are either other Digest objects or
> Strings
> fun toString(): String // e.g. for debugging purposes
> fun compareTo(other: Digest): Int // e.g. for debugging purposes.
> }
> {code}
> Then the digest for RexCall could be the bits relevant to RexCall itself +
> digests of the operands (which can be reused as is)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Reopened] (CALCITE-3786) Add Digest interface to enable efficient hashCode(equals) for RexNode and RelNode
[
https://issues.apache.org/jira/browse/CALCITE-3786?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon reopened CALCITE-3786:
-
> Add Digest interface to enable efficient hashCode(equals) for RexNode and
> RelNode
> -
>
> Key: CALCITE-3786
> URL: https://issues.apache.org/jira/browse/CALCITE-3786
> Project: Calcite
> Issue Type: New Feature
> Components: core
>Affects Versions: 1.21.0
>Reporter: Vladimir Sitnikov
>Assignee: Danny Chen
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 4h
> Remaining Estimate: 0h
>
> Current digests for RexNode, RelNode, RelType, and similar cases use String
> concatenation.
> It is easy to implement, however, it has drawbacks:
> 1) String objects cannot be reused. For instance, RexCall has operands,
> however, the digest is duplicated. It causes extra memory use and extra CPU
> for string copying
> 2) There's no way to have multiple #toString() methods. RelType might need
> multiple digests: "including field names", "excluding field names".
> A suggested resolution might be behind the lines of
> {code:java}
> class Digest { // immutable
> final int hashCode; // speedup hashCode and equals
> final Object[] contents; // The values are either other Digest objects or
> Strings
> String toString(); // e.g. for debugging purposes
> int compareTo(Digest); // e.g. for debugging purposes.
> }
> {code}
> Note how fields in Kotlin are aligned much better, and it makes it easier to
> read:
> {code:java}
> class Digest { // immutable
> val hashCode: Int // speedup hashCode and equals
> val contents: Array // The values are either other Digest objects or
> Strings
> fun toString(): String // e.g. for debugging purposes
> fun compareTo(other: Digest): Int // e.g. for debugging purposes.
> }
> {code}
> Then the digest for RexCall could be the bits relevant to RexCall itself +
> digests of the operands (which can be reused as is)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
[
https://issues.apache.org/jira/browse/CALCITE-4025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17117160#comment-17117160
]
Laurent Goujon commented on CALCITE-4025:
-
Thanks for helping me on the the test case, I'll try to implement your
suggestion.
> DelegatingScope#fullyQualify may exit too soon when resolving table alias
> -
>
> Key: CALCITE-4025
> URL: https://issues.apache.org/jira/browse/CALCITE-4025
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Priority: Major
>
> When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
> there's a look going over all the possible sub prefixes and trying to resolve
> them, until one succeed. But if not, the same lookup is done using a liberal
> name matcher. If that one resolves, then the method exit early with an
> exception message instead of going over the whole loop or trying for a fully
> qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Assigned] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
[
https://issues.apache.org/jira/browse/CALCITE-4025?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon reassigned CALCITE-4025:
---
Assignee: Laurent Goujon
> DelegatingScope#fullyQualify may exit too soon when resolving table alias
> -
>
> Key: CALCITE-4025
> URL: https://issues.apache.org/jira/browse/CALCITE-4025
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>
> When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
> there's a look going over all the possible sub prefixes and trying to resolve
> them, until one succeed. But if not, the same lookup is done using a liberal
> name matcher. If that one resolves, then the method exit early with an
> exception message instead of going over the whole loop or trying for a fully
> qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
[
https://issues.apache.org/jira/browse/CALCITE-4025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17117149#comment-17117149
]
Laurent Goujon commented on CALCITE-4025:
-
I mentioned it as a possible issue on the Calcite mailing list but didn't get
any feedback on it:
https://mail-archives.apache.org/mod_mbox/calcite-dev/202005.mbox/%3CCAGQXjMwWr%2Bot_-rjTzo6dG1are3DM%3DWp3vQYNsB640JTSGgLbQ%40mail.gmail.com%3E
> DelegatingScope#fullyQualify may exit too soon when resolving table alias
> -
>
> Key: CALCITE-4025
> URL: https://issues.apache.org/jira/browse/CALCITE-4025
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Priority: Major
>
> When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
> there's a look going over all the possible sub prefixes and trying to resolve
> them, until one succeed. But if not, the same lookup is done using a liberal
> name matcher. If that one resolves, then the method exit early with an
> exception message instead of going over the whole loop or trying for a fully
> qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
[
https://issues.apache.org/jira/browse/CALCITE-4025?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-4025:
Issue Type: Bug (was: Improvement)
> DelegatingScope#fullyQualify may exit too soon when resolving table alias
> -
>
> Key: CALCITE-4025
> URL: https://issues.apache.org/jira/browse/CALCITE-4025
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Priority: Major
>
> When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
> there's a look going over all the possible sub prefixes and trying to resolve
> them, until one succeed. But if not, the same lookup is done using a liberal
> name matcher. If that one resolves, then the method exit early with an
> exception message instead of going over the whole loop or trying for a fully
> qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (CALCITE-4025) DelegatingScope#fullyQualify may exit too soon when resolving table alias
Laurent Goujon created CALCITE-4025:
---
Summary: DelegatingScope#fullyQualify may exit too soon when
resolving table alias
Key: CALCITE-4025
URL: https://issues.apache.org/jira/browse/CALCITE-4025
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Laurent Goujon
When fully qualifing an id with {{DelegatingScope#fullyQualify()}} method,
there's a look going over all the possible sub prefixes and trying to resolve
them, until one succeed. But if not, the same lookup is done using a liberal
name matcher. If that one resolves, then the method exit early with an
exception message instead of going over the whole loop or trying for a fully
qualified table.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3996) Fix NPE when query syntax is invalid
[ https://issues.apache.org/jira/browse/CALCITE-3996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1710#comment-1710 ] Laurent Goujon commented on CALCITE-3996: - Can you please provide a more detailed description? > Fix NPE when query syntax is invalid > > > Key: CALCITE-3996 > URL: https://issues.apache.org/jira/browse/CALCITE-3996 > Project: Calcite > Issue Type: Bug >Reporter: James Kim >Priority: Critical > > Fix NPE when query syntax is invalid -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3956) Unify comparison logic for RelOptCost
[
https://issues.apache.org/jira/browse/CALCITE-3956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101806#comment-17101806
]
Laurent Goujon commented on CALCITE-3956:
-
I'm dubious about this change: Like Julian, I'm not sure the actual code use
{{RelOptCost}} would change much BUT at the same time, adding a {{compareCost}}
method might let people think that there's a total ordering where there is none.
(The change also breaks API for those projects which implements their own
{{RelOptCost}})
> Unify comparison logic for RelOptCost
> -
>
> Key: CALCITE-3956
> URL: https://issues.apache.org/jira/browse/CALCITE-3956
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Liya Fan
>Assignee: Liya Fan
>Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Currently, comparisons between RelOptCost objects are based on 3 methods:
> 1. {{boolean isLe(RelOptCost cost)}}
> 2. {{boolean isLt(RelOptCost cost)}}
> 3. {{boolean equals(RelOptCost cost)}}
> The 3 methods used in combination determine the relation between RelOptCost
> objects.
> There are some problems with this implementation:
> 1. Some logic is duplicate in the above methods, making it difficult to
> maintain.
> 2. To determine the relation between RelOptCost objects, we often need to
> call more than one comparison methods, leading to performance overhead.
> 3. Since the logic is spread in multiple methods, it is easy to end up with
> contradictive comparison logic, which will suprise the users. For example,
> the following assertion should hold according to common sense:
> {{if a >=b, then we have a > b or a == b}}
> However, with the current implementation of {{VolcanoCost}}, we can easily
> create instances that violate the above assertion.
> To solve the problems, we want to make {{RelOptCost}} extends the
> {{Comparable}}, so the comparison logic is unified in the
> {{compareTo}} method, which solves the above problems.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Resolved] (CALCITE-3965) Excessive time waiting on DiffRepository lock
[
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-3965.
-
Fix Version/s: 1.23.0
Resolution: Fixed
Changed merged as
[0220166|https://github.com/apache/calcite/commit/022016611091fdc021686aaf1a44f5217b4b3d30]
> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Fix For: 1.23.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized
> and gradle/junit tries to use as many cores as possible (16 on my machine).
> But the tests take a very long time, approximatevely 90minutes on my machine,
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that
> most of them are waiting on {{DiffRepository}} methods
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained
> the lock (and is usually flushing data on disk).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock
[
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101202#comment-17101202
]
Laurent Goujon commented on CALCITE-3965:
-
Sounds reasonable
> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized
> and gradle/junit tries to use as many cores as possible (16 on my machine).
> But the tests take a very long time, approximatevely 90minutes on my machine,
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that
> most of them are waiting on {{DiffRepository}} methods
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained
> the lock (and is usually flushing data on disk).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3517) DiffRepository spends too much time writing XML, makes some tests 5x slower
[
https://issues.apache.org/jira/browse/CALCITE-3517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101182#comment-17101182
]
Laurent Goujon commented on CALCITE-3517:
-
I'm proposing a patch to create a JUnit extension to manage {{DiffRepository}}
lifecycle so that the file is only dumped once all the tests are run:
https://github.com/apache/calcite/pull/1962
> DiffRepository spends too much time writing XML, makes some tests 5x slower
> ---
>
> Key: CALCITE-3517
> URL: https://issues.apache.org/jira/browse/CALCITE-3517
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Tests that use {{DiffRepository}} are spending far too much effort writing
> XML, even if the XML matches the reference file. For example, If I comment
> out [a call to set(tag,
> next)|https://github.com/apache/calcite/blob/ee83efd360793ef4201f4cdfc2af8d837b76ca69/core/src/test/java/org/apache/calcite/test/DiffRepository.java#L267],
> {{RelOptRulesTest}} improves from 32s to 6s; {{SqlToRelConverterTest}}
> improves from 24s to 4.7s; {{SqlPrettyWriterTest}} remains .8s.
> The {{DiffRepository.expand}} method is the cause of the inefficiency. It
> causes the entire XML document to be re-generated and written to disk. This
> is not just slow but quadratic - if a test has N cases, each test writes the
> XML document, an effort proportional to N.
> {{DiffRepository}} should remain conservative. If one of the tests fails, and
> a later test crashes, the output from the failed test should have been
> written out. It is acceptable if the test remains slow if there are test
> failures.
> {{DiffRepository}} is only used in tests; this bug does not affect production
> code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock
[
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101161#comment-17101161
]
Laurent Goujon commented on CALCITE-3965:
-
I guess after all this time, I still don't know how the JIRA/Github integration
works: https://github.com/apache/calcite/pull/1954
> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized
> and gradle/junit tries to use as many cores as possible (16 on my machine).
> But the tests take a very long time, approximatevely 90minutes on my machine,
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that
> most of them are waiting on {{DiffRepository}} methods
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained
> the lock (and is usually flushing data on disk).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock
[
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17097079#comment-17097079
]
Laurent Goujon commented on CALCITE-3965:
-
I agree with the unnecessary xml writing, but adding a synchronized keyword
when it is not necessary (which is my analysis, and maybe I wrong here, so I
would appreciate a second pair of eyes) is also a cause for lock contention.
> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized
> and gradle/junit tries to use as many cores as possible (16 on my machine).
> But the tests take a very long time, approximatevely 90minutes on my machine,
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that
> most of them are waiting on {{DiffRepository}} methods
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained
> the lock (and is usually flushing data on disk).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock
[
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17097071#comment-17097071
]
Laurent Goujon commented on CALCITE-3965:
-
This is somehow related to CALCITE-3517, but after a look at the code,
{{DiffRepository#expand}} do not do write to disk, and the main issue is really
contention around the instance lock. The method is synchronized, but most
operations look thread-safe. They are some calls to get and set (which are also
synchronized), but it doesn't look like they need to done "atomically".
Removing {{synchronized}} on the expand() method results in the build
completing in 2min30s with no test failures.
> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>
> When running the whole test suite from commandline, tests are parallelized
> and gradle/junit tries to use as many cores as possible (16 on my machine).
> But the tests take a very long time, approximatevely 90minutes on my machine,
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that
> most of them are waiting on {{DiffRepository}} methods
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained
> the lock (and is usually flushing data on disk).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (CALCITE-3965) Excessive time waiting on DiffRepository lock
Laurent Goujon created CALCITE-3965:
---
Summary: Excessive time waiting on DiffRepository lock
Key: CALCITE-3965
URL: https://issues.apache.org/jira/browse/CALCITE-3965
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Assignee: Laurent Goujon
When running the whole test suite from commandline, tests are parallelized and
gradle/junit tries to use as many cores as possible (16 on my machine). But the
tests take a very long time, approximatevely 90minutes on my machine, and
several of them failed because they took too long to complete.
Using jstack to look at the threads state while tests are running show that
most of them are waiting on {{DiffRepository}} methods
({{DiffRepository#expand}} in most cases) while one of the thread obtained the
lock (and is usually flushing data on disk).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3187) Derive all decimal return type through type factory
[
https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16897591#comment-16897591
]
Laurent Goujon commented on CALCITE-3187:
-
According to Travis CI, {{-Werror}} is not causing any issue for all JDK
versions, so pushing the change to master
> Derive all decimal return type through type factory
> ---
>
> Key: CALCITE-3187
> URL: https://issues.apache.org/jira/browse/CALCITE-3187
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Praveen Kumar Desabandu
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.21.0
>
> Time Spent: 4.5h
> Remaining Estimate: 0h
>
> Currently decimal product and quotient return types are derived through type
> factory, this allows clients to override the return type if they so desire.
> But decimal sum is embedded in return types, also decimal mod does not have a
> return type inference defined.
> This task is to derive all of the return types through type factory, so that
> clients can override if they wish to.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3187) Derive all decimal return type through type factory
[ https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16897568#comment-16897568 ] Laurent Goujon commented on CALCITE-3187: - Here's a pull request to address compiler warnings (including those for CALCITE-3031): https://github.com/apache/calcite/pull/1344 > Derive all decimal return type through type factory > --- > > Key: CALCITE-3187 > URL: https://issues.apache.org/jira/browse/CALCITE-3187 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Praveen Kumar Desabandu >Assignee: Laurent Goujon >Priority: Major > Labels: pull-request-available > Fix For: 1.21.0 > > Time Spent: 4.5h > Remaining Estimate: 0h > > Currently decimal product and quotient return types are derived through type > factory, this allows clients to override the return type if they so desire. > But decimal sum is embedded in return types, also decimal mod does not have a > return type inference defined. > This task is to derive all of the return types through type factory, so that > clients can override if they wish to. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (CALCITE-3187) Derive all decimal return type through type factory
[
https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16897552#comment-16897552
]
Laurent Goujon commented on CALCITE-3187:
-
For {{RelDataTypeFactoryImpl}}, shouldn't I suppress the warning instead by
adding {{@Deprecated}} to these methods? They were already existing before the
patch, it just happened I added {{@Deprecated}} to the interface, but their
body do not use deprecated code.
It would be nice also if the compiler could be configured to error for
warnings, instead of just printing out if we have no tolerance for those.
> Derive all decimal return type through type factory
> ---
>
> Key: CALCITE-3187
> URL: https://issues.apache.org/jira/browse/CALCITE-3187
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Praveen Kumar Desabandu
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.21.0
>
> Time Spent: 4.5h
> Remaining Estimate: 0h
>
> Currently decimal product and quotient return types are derived through type
> factory, this allows clients to override the return type if they so desire.
> But decimal sum is embedded in return types, also decimal mod does not have a
> return type inference defined.
> This task is to derive all of the return types through type factory, so that
> clients can override if they wish to.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Comment Edited] (CALCITE-3216) ClassCastException when running window function over union
[
https://issues.apache.org/jira/browse/CALCITE-3216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16893969#comment-16893969
]
Laurent Goujon edited comment on CALCITE-3216 at 7/26/19 4:32 PM:
--
The problem is likely to be common to any set operator: if the children of the
operator are compatible but not exactly the same, the operator row type is the
least restrictive of all the input types. But as far as I can tell, during
execution, Calcite execution engine doesn't coerce the the records generated by
the operator to match the row type, causing issues for parent operators as
their physical representation do not match the expected type.
It's easy to spot in the generated source code:
{code:java}
/* 1 */ public org.apache.calcite.linq4j.Enumerable bind(final
org.apache.calcite.DataContext root) {
/* 2 */ final org.apache.calcite.linq4j.Enumerable _inputEnumerable =
org.apache.calcite.linq4j.Linq4j.asEnumerable(new Integer[] {
/* 3 */ 0});
/* 4 */ final org.apache.calcite.linq4j.AbstractEnumerable child0 = new
org.apache.calcite.linq4j.AbstractEnumerable(){
/* 5 */ public org.apache.calcite.linq4j.Enumerator enumerator() {
/* 6 */ return new org.apache.calcite.linq4j.Enumerator(){
/* 7 */ public final org.apache.calcite.linq4j.Enumerator
inputEnumerator = _inputEnumerable.enumerator();
/* 8 */ public void reset() {
/* 9 */ inputEnumerator.reset();
/* 10 */ }
/* 11 */
/* 12 */ public boolean moveNext() {
/* 13 */ return inputEnumerator.moveNext();
/* 14 */ }
/* 15 */
/* 16 */ public void close() {
/* 17 */ inputEnumerator.close();
/* 18 */ }
/* 19 */
/* 20 */ public Object current() {
/* 21 */ return (byte)1;
/* 22 */ }
/* 23 */
/* 24 */ };
/* 25 */ }
/* 26 */
/* 27 */ };
/* 28 */ int prevStart;
/* 29 */ int prevEnd;
/* 30 */ final java.util.Comparator comparator = new java.util.Comparator(){
/* 31 */ public int compare(Integer v0, Integer v1) {
/* 32 */ int c;
/* 33 */ return 0;
/* 34 */ }
/* 35 */
/* 36 */ public int compare(Object o0, Object o1) {
/* 37 */ return this.compare((Integer) o0, (Integer) o1);
/* 38 */ }
/* 39 */
/* 40 */ };
/* 41 */ final org.apache.calcite.runtime.SortedMultiMap multiMap = new
org.apache.calcite.runtime.SortedMultiMap();
/* 42 */ child0.union(org.apache.calcite.linq4j.Linq4j.asEnumerable(new
Integer[] {
/* 43 */ 2})).foreach(new org.apache.calcite.linq4j.function.Function1() {
/* 44 */ public Object apply(int v) {
/* 45 */ int key = v;
/* 46 */ multiMap.putMulti(key, v);
/* 47 */ return null;
/* 48 */ }
/* 49 */ public Object apply(Integer v) {
/* 50 */ return apply(
/* 51 */ v.intValue());
/* 52 */ }
/* 53 */ public Object apply(Object v) {
/* 54 */ return apply(
/* 55 */ (Integer) v);
/* 56 */ }
/* 57 */ }
/* 58 */ );
/* 59 */ final java.util.Iterator iterator = multiMap.arrays(comparator);
/* 60 */ final java.util.ArrayList _list = new java.util.ArrayList(
/* 61 */ multiMap.size());
/* 62 */ Long COUNTa0w0 = (Long) null;
/* 63 */ while (iterator.hasNext()) {
/* 64 */ final Object[] _rows = (Object[]) iterator.next();
/* 65 */ prevStart = -1;
/* 66 */ prevEnd = 2147483647;
/* 67 */ final int maxX = _rows.length - 1;
/* 68 */ for (int i = 0; i < _rows.length; ++i) {
/* 69 */ if (maxX != prevEnd) {
/* 70 */ int actualStart = maxX < prevEnd ? 0 : prevEnd + 1;
/* 71 */ prevEnd = maxX;
/* 72 */ COUNTa0w0 = Long.valueOf(maxX + 1);
/* 73 */ }
/* 74 */ _list.add(new Object[] {
/* 75 */ org.apache.calcite.runtime.SqlFunctions.toInt(_rows[i]),
/* 76 */ COUNTa0w0});
/* 77 */ }
/* 78 */ }
/* 79 */ multiMap.clear();
/* 80 */ return org.apache.calcite.linq4j.Linq4j.asEnumerable(_list);
/* 81 */ }
/* 82 */
/* 83 */
/* 84 */ public Class getElementType() {
/* 85 */ return java.lang.Object[].class;
/* 86 */ }
{code}
Alas, I'm not familiar at all with the execution engine, and have no idea what
the fix should look like.
was (Author: laurentgo):
The problem is likely to be common to any set operator: if the children of the
operator are compatible but not exactly the same, the operator row type is the
least restrictive of all the input types. But as far as I can tell, during
execution, Calcite execution engine doesn't coerce the the records generated by
the operator to match the row type, causing issues for parent operators as
their physical representation do not match the expected type.
It's easy to spot in the generated source code:
{code:java}
public
[jira] [Commented] (CALCITE-3216) ClassCastException when running window function over union
[
https://issues.apache.org/jira/browse/CALCITE-3216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16893969#comment-16893969
]
Laurent Goujon commented on CALCITE-3216:
-
The problem is likely to be common to any set operator: if the children of the
operator are compatible but not exactly the same, the operator row type is the
least restrictive of all the input types. But as far as I can tell, during
execution, Calcite execution engine doesn't coerce the the records generated by
the operator to match the row type, causing issues for parent operators as
their physical representation do not match the expected type.
It's easy to spot in the generated source code:
{code:java}
public org.apache.calcite.linq4j.Enumerable bind(final
org.apache.calcite.DataContext root) {
final org.apache.calcite.linq4j.Enumerable _inputEnumerable =
org.apache.calcite.linq4j.Linq4j.asEnumerable(new Integer[] {
0});
final org.apache.calcite.linq4j.AbstractEnumerable child0 = new
org.apache.calcite.linq4j.AbstractEnumerable(){
public org.apache.calcite.linq4j.Enumerator enumerator() {
return new org.apache.calcite.linq4j.Enumerator(){
public final org.apache.calcite.linq4j.Enumerator inputEnumerator =
_inputEnumerable.enumerator();
public void reset() {
inputEnumerator.reset();
}
public boolean moveNext() {
return inputEnumerator.moveNext();
}
public void close() {
inputEnumerator.close();
}
public Object current() {
return (byte)1;
}
};
}
};
final org.apache.calcite.linq4j.Enumerable _inputEnumerable0 =
child0.union(org.apache.calcite.linq4j.Linq4j.asEnumerable(new Integer[] {
2}));
return new org.apache.calcite.linq4j.AbstractEnumerable(){
public org.apache.calcite.linq4j.Enumerator enumerator() {
return new org.apache.calcite.linq4j.Enumerator(){
public final org.apache.calcite.linq4j.Enumerator inputEnumerator
= _inputEnumerable0.enumerator();
public void reset() {
inputEnumerator.reset();
}
public boolean moveNext() {
return inputEnumerator.moveNext();
}
public void close() {
inputEnumerator.close();
}
public Object current() {
return
org.apache.calcite.runtime.SqlFunctions.toInt(inputEnumerator.current()) * 2;
}
};
}
};
}
public Class getElementType() {
return int.class;
}
{code}
Alas, I'm not familiar at all with the execution engine, and have no idea what
the fix should look like.
> ClassCastException when running window function over union
> --
>
> Key: CALCITE-3216
> URL: https://issues.apache.org/jira/browse/CALCITE-3216
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Priority: Major
>
> I discovered an issue in Calcite execution engine which can be captured by
> this simple query:
> {code:sql}
> select *, count(*) over (partition by "id") from (
> select "id" from (VALUES(CAST(1 AS TINYINT))) "foo"("id")
> union
> select "id" from (VALUES(2)) "foo"("id"))
> {code}
> When running this query using JdbcTest for example, I got the following
> stacktrace:
> {noformat}
> Caused by: java.lang.ClassCastException: java.lang.Byte cannot be cast to
> java.lang.Integer
> at Baz$3.apply(ANONYMOUS.java:55)
> at
> org.apache.calcite.linq4j.DefaultEnumerable.foreach(DefaultEnumerable.java:77)
> at Baz.bind(Baz.java:43)
> at
> org.apache.calcite.jdbc.CalcitePrepare$CalciteSignature.enumerable(CalcitePrepare.java:355)
> at
> org.apache.calcite.jdbc.CalciteConnectionImpl.enumerable(CalciteConnectionImpl.java:316)
> at
> org.apache.calcite.jdbc.CalciteMetaImpl._createIterable(CalciteMetaImpl.java:506)
> at
> org.apache.calcite.jdbc.CalciteMetaImpl.createIterable(CalciteMetaImpl.java:497)
> at
> org.apache.calcite.avatica.AvaticaResultSet.execute(AvaticaResultSet.java:182)
> at
> org.apache.calcite.jdbc.CalciteResultSet.execute(CalciteResultSet.java:64)
> at
> org.apache.calcite.jdbc.CalciteResultSet.execute(CalciteResultSet.java:1)
> at
> org.apache.calcite.avatica.AvaticaConnection$1.execute(AvaticaConnection.java:667)
> at
> org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(CalciteMetaImpl.java:566)
> at
> org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:675)
> at
>
[jira] [Created] (CALCITE-3216) ClassCastException when running window function over union
Laurent Goujon created CALCITE-3216:
---
Summary: ClassCastException when running window function over union
Key: CALCITE-3216
URL: https://issues.apache.org/jira/browse/CALCITE-3216
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
I discovered an issue in Calcite execution engine which can be captured by this
simple query:
{code:sql}
select *, count(*) over (partition by "id") from (
select "id" from (VALUES(CAST(1 AS TINYINT))) "foo"("id")
union
select "id" from (VALUES(2)) "foo"("id"))
{code}
When running this query using JdbcTest for example, I got the following
stacktrace:
{noformat}
Caused by: java.lang.ClassCastException: java.lang.Byte cannot be cast to
java.lang.Integer
at Baz$3.apply(ANONYMOUS.java:55)
at
org.apache.calcite.linq4j.DefaultEnumerable.foreach(DefaultEnumerable.java:77)
at Baz.bind(Baz.java:43)
at
org.apache.calcite.jdbc.CalcitePrepare$CalciteSignature.enumerable(CalcitePrepare.java:355)
at
org.apache.calcite.jdbc.CalciteConnectionImpl.enumerable(CalciteConnectionImpl.java:316)
at
org.apache.calcite.jdbc.CalciteMetaImpl._createIterable(CalciteMetaImpl.java:506)
at
org.apache.calcite.jdbc.CalciteMetaImpl.createIterable(CalciteMetaImpl.java:497)
at
org.apache.calcite.avatica.AvaticaResultSet.execute(AvaticaResultSet.java:182)
at
org.apache.calcite.jdbc.CalciteResultSet.execute(CalciteResultSet.java:64)
at
org.apache.calcite.jdbc.CalciteResultSet.execute(CalciteResultSet.java:1)
at
org.apache.calcite.avatica.AvaticaConnection$1.execute(AvaticaConnection.java:667)
at
org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(CalciteMetaImpl.java:566)
at
org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:675)
at
org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:156)
{noformat}
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Resolved] (CALCITE-3187) Derive all decimal return type through type factory
[ https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Laurent Goujon resolved CALCITE-3187. - Resolution: Fixed Fix Version/s: 1.21.0 Fixed with commit [f7c0b0a|https://github.com/apache/calcite/commit/f7c0b0a18ab72338e0b3afcdfd087aab3572fddb] (+followup commit to fix javadoc typo: [8c0d792|https://github.com/apache/calcite/commit/8c0d792dd735eded2bb11be0c7c195b8f302ae90]) > Derive all decimal return type through type factory > --- > > Key: CALCITE-3187 > URL: https://issues.apache.org/jira/browse/CALCITE-3187 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Praveen Kumar Desabandu >Assignee: Laurent Goujon >Priority: Major > Labels: pull-request-available > Fix For: 1.21.0 > > Time Spent: 4.5h > Remaining Estimate: 0h > > Currently decimal product and quotient return types are derived through type > factory, this allows clients to override the return type if they so desire. > But decimal sum is embedded in return types, also decimal mod does not have a > return type inference defined. > This task is to derive all of the return types through type factory, so that > clients can override if they wish to. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Assigned] (CALCITE-3187) Derive all decimal return type through type factory
[ https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Laurent Goujon reassigned CALCITE-3187: --- Assignee: Laurent Goujon > Derive all decimal return type through type factory > --- > > Key: CALCITE-3187 > URL: https://issues.apache.org/jira/browse/CALCITE-3187 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Praveen Kumar Desabandu >Assignee: Laurent Goujon >Priority: Major > Labels: pull-request-available > Time Spent: 4.5h > Remaining Estimate: 0h > > Currently decimal product and quotient return types are derived through type > factory, this allows clients to override the return type if they so desire. > But decimal sum is embedded in return types, also decimal mod does not have a > return type inference defined. > This task is to derive all of the return types through type factory, so that > clients can override if they wish to. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (CALCITE-3187) Derive all decimal return type through type factory
[ https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16885548#comment-16885548 ] Laurent Goujon commented on CALCITE-3187: - Will do. > Derive all decimal return type through type factory > --- > > Key: CALCITE-3187 > URL: https://issues.apache.org/jira/browse/CALCITE-3187 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Praveen Kumar Desabandu >Priority: Major > Labels: pull-request-available > Time Spent: 3h 10m > Remaining Estimate: 0h > > Currently decimal product and quotient return types are derived through type > factory, this allows clients to override the return type if they so desire. > But decimal sum is embedded in return types, also decimal mod does not have a > return type inference defined. > This task is to derive all of the return types through type factory, so that > clients can override if they wish to. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (CALCITE-3187) Derive all decimal return type through type factory
[ https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16883256#comment-16883256 ] Laurent Goujon commented on CALCITE-3187: - Correct, my bad. Sounds reasonable to me. [~praveenbingo] is that acceptable to you? > Derive all decimal return type through type factory > --- > > Key: CALCITE-3187 > URL: https://issues.apache.org/jira/browse/CALCITE-3187 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Praveen Kumar Desabandu >Priority: Major > Labels: pull-request-available > Time Spent: 2h 20m > Remaining Estimate: 0h > > Currently decimal product and quotient return types are derived through type > factory, this allows clients to override the return type if they so desire. > But decimal sum is embedded in return types, also decimal mod does not have a > return type inference defined. > This task is to derive all of the return types through type factory, so that > clients can override if they wish to. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (CALCITE-3183) Trimming method for Filter rel uses wrong traitSet
[
https://issues.apache.org/jira/browse/CALCITE-3183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16883185#comment-16883185
]
Laurent Goujon commented on CALCITE-3183:
-
why not using {{Iterable}} for the factory too? it would avoid unnecessary
copies (until the constructor is called and do {{ImmutableList.copyOf()}})
> Trimming method for Filter rel uses wrong traitSet
> ---
>
> Key: CALCITE-3183
> URL: https://issues.apache.org/jira/browse/CALCITE-3183
> Project: Calcite
> Issue Type: Bug
>Reporter: Juhwan Kim
>Assignee: Juhwan Kim
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> It seems like there is a bug here:
> https://github.com/apache/calcite/blob/e8d598a434e8dbadaf756f8c57c748f4d7e16fdf/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L487.
> Unlike other trimming methods, filter trim function copies the current filter
> rel and directly pushes it to the builder instead of calling factory method
> for filter rel. The problem with the current code is that it uses the same
> traitSet even though it would no longer be valid after trimming its input.
> For example, fields in collation might have been updated after trimming. We
> should reflect this change when creating a new rel.
>
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3187) Derive all decimal return type through type factory
[
https://issues.apache.org/jira/browse/CALCITE-3187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16882235#comment-16882235
]
Laurent Goujon commented on CALCITE-3187:
-
I was wondering about this. It seems that long time (6year) ago, methods
{{createDecimalProduct}}/{{createDecimalQuotient}} were added to
{{RelDataTypeFactory}}. But later on (2year ago), several methods were added to
{{RelDataTypeFactory}} to derive return type for agg functions (like
{{deriveSum}}).
Should we add the new {{createDecimal}} methods to {{RelDataTypeFactory}} for
consistency, and to {{RelDataTypeFactory}} which seems to be the new place for
deriving return types. Maybe the original methods can be moved too (with
deprecation and redirecting to the type system?)
> Derive all decimal return type through type factory
> ---
>
> Key: CALCITE-3187
> URL: https://issues.apache.org/jira/browse/CALCITE-3187
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Praveen Kumar Desabandu
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> Currently decimal product and quotient return types are derived through type
> factory, this allows clients to override the return type if they so desire.
> But decimal sum is embedded in return types, also decimal mod does not have a
> return type inference defined.
> This task is to derive all of the return types through type factory, so that
> clients can override if they wish to.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3111) Allow custom implementations of Correlate in RelDecorrelator
[ https://issues.apache.org/jira/browse/CALCITE-3111?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862317#comment-16862317 ] Laurent Goujon commented on CALCITE-3111: - Working in the same team as [~Juhwan], yes, it is primarily to allow different types of logical nodes. That said, I believe it is up to anybody to decide when they want to use RelDecorrelator. > Allow custom implementations of Correlate in RelDecorrelator > > > Key: CALCITE-3111 > URL: https://issues.apache.org/jira/browse/CALCITE-3111 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Juhwan Kim >Assignee: Juhwan Kim >Priority: Minor > > Currently, RelDecorrelator code only works for LogicalCorrelate. > Decorrelating through Calcite would become much more flexible if it allows > using custom implementations of Correlate. This would require refactoring all > logical rels used in RelDecorrelator to the abstract ones(e.g > LogicalCorrelate -> Correlate). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-3111) Allow custom implementations of Correlate in RelDecorrelator
[
https://issues.apache.org/jira/browse/CALCITE-3111?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862320#comment-16862320
]
Laurent Goujon commented on CALCITE-3111:
-
{quote}
I guess it would be convenient to generalize RelDecorrelator to work with other
kind of RelNodes (apart from Logical ones). Correct if I am wrong but this
applies to all relational expressions used in the decorrelator (not only
Correlate and LogicalCorrelate).
{quote}
That's the idea: refactor {{RelDecorrelator}} to use {{RelBuilder}} + base
Calcite classes ({{Project}}, {{Filter}}, {{Correlate}}, ...) instead of
{{LogicalXXX}} nodes directly...
> Allow custom implementations of Correlate in RelDecorrelator
>
>
> Key: CALCITE-3111
> URL: https://issues.apache.org/jira/browse/CALCITE-3111
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Juhwan Kim
>Assignee: Juhwan Kim
>Priority: Minor
>
> Currently, RelDecorrelator code only works for LogicalCorrelate.
> Decorrelating through Calcite would become much more flexible if it allows
> using custom implementations of Correlate. This would require refactoring all
> logical rels used in RelDecorrelator to the abstract ones(e.g
> LogicalCorrelate -> Correlate).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (CALCITE-3082) NPE in SqlUtil#getSelectListItem
[
https://issues.apache.org/jira/browse/CALCITE-3082?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-3082.
-
Resolution: Fixed
Fix Version/s: 1.20.0
Fixed in
https://github.com/apache/calcite/commit/d8768f9c07fa3927475902e27c13c8bc39687897
> NPE in SqlUtil#getSelectListItem
>
>
> Key: CALCITE-3082
> URL: https://issues.apache.org/jira/browse/CALCITE-3082
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Queries similar to {{SELECT 1 UNION SELECT 2, 3}} causes validator to throws
> a {{NullPointerException}} instance instead of a proper error message about
> the column count mismatch.
> {{SqlUtil#getSelectListItem}} is used by Set operators to get select items if
> the types of operands don't match. Unfortunately the method may throw an NPE
> exception if one of the operand doesn't have a FROM clause.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3101) PushDownJoinConditions is not always a valid transformation
[
https://issues.apache.org/jira/browse/CALCITE-3101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16852036#comment-16852036
]
Laurent Goujon commented on CALCITE-3101:
-
{quote}
Laurent Goujon, I agree, but whether we transform the SqlNode tree or just
control the SQL that is generated from it is a just a matter of tactics. Note
that SqlDialect sometimes helps with the former (e.g.
SqlDialect.emulateNullDirection returns a transformed SqlNode).
{quote}
Are we talking about the {{SqlNode}} tree generated by {{RelToSqlConverter}}?
If yes, that seems reasonable. But if we are talking about the original
{{SqlNode}} tree which is feed to {{SqlToRelConverter}}, then I'm not sure if
this is entirely true based on recent experiences like {{IS NOT DISTINCT FROM}}
where it's much harder to optimize the query when using the {{CASE}} expression
form, and rules have to try to normalize back the expression to correctly
identify push downs.
Otherwise, the generated SQL looks good to me.
> PushDownJoinConditions is not always a valid transformation
> ---
>
> Key: CALCITE-3101
> URL: https://issues.apache.org/jira/browse/CALCITE-3101
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.16.0
> Environment: Java app pointing to Oracle database.
>Reporter: Paul Jackson
>Priority: Major
>
> SqlToRelConverter can create a plan that is invalid when converted back to
> SQL in cases where the expression that is pushed to the projection returns a
> Boolean. The following example pushes IS NOT NULL to a select. Several SQL
> dialects do not support this. Oracle, for example, sees IS NOT NULL as a
> condition rather than an expression. It returns a Boolean data type, which is
> not supported. Likewise, Microsoft SQL Server does not support IS NOT NULL in
> a projection expression.
> Steps to reproduce (Oracle):
> DDL:
> {code:java}
> CREATE TABLE "EMP" (
> "empno" INTEGER PRIMARY KEY NOT NULL,
> "ename" VARCHAR(100),
> "deptno" INTEGER);{code}
> Start with this query:
> {code:java}
> SELECT "EMP"."empno", "t"."ename" "ename0"
> FROM "EMP"
> INNER JOIN "EMP" "t"
> ON "EMP"."deptno" = "t"."deptno" AND "t"."ename" IS NOT NULL{code}
> Parse using {{SqlToRelConverter.convertQuery()}}. At this point in the stack
> trace:
> {noformat}
> org.apache.calcite.plan.RelOptUtil.pushDownJoinConditions(RelOptUtil.java:3222)
> org.apache.calcite.sql2rel.SqlToRelConverter.createJoin(SqlToRelConverter.java:2414)
> org.apache.calcite.sql2rel.SqlToRelConverter.convertFrom(SqlToRelConverter.java:2056)
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:641)
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:622)
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3057)
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:558)
> {noformat}
> the {{RelNode}} is:
> {noformat}
> LogicalJoin(condition=[AND(=($3, $8), IS NOT NULL($6))], joinType=[inner])
> JdbcTableScan(table=[[XYZ, EMP]])
> JdbcTableScan(table=[[XYZ, EMP]])
> {noformat}
> After {{pushDownJoinConditions}} the {{RelNode}} is:
> {noformat}
> LogicalJoin(condition=[AND(=($3, $8), $10)], joinType=[inner])
> JdbcTableScan(table=[[XYZ, EMP]])
> LogicalProject(empno=[$0], ename=[$1], job=[$2], deptno=[$3], etype=[$4],
> $f5=[IS NOT NULL($1)])
> JdbcTableScan(table=[[XYZ, EMP]])
> {noformat}
> Which leads to invalid SQL ("ORA-00923: FROM keyword not found where
> expected"):
> {code:java}
> SELECT "EMP"."empno", "t"."ename" "ename0"
> FROM "XYZ"."EMP"
> INNER JOIN (
> SELECT "empno", "ename", "job", "deptno", "etype", "ename" IS NOT NULL
> "$f5"
> FROM "XYZ"."EMP") "t"
> ON "EMP"."deptno" = "t"."deptno" AND "t"."$f5"
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3101) PushDownJoinConditions is not always a valid transformation
[
https://issues.apache.org/jira/browse/CALCITE-3101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16851485#comment-16851485
]
Laurent Goujon commented on CALCITE-3101:
-
One could argue that in Calcite dialect/Rel tree, {{IS NOT NULL}} is a valid
operator, but it should be the responsability of the target {{SqlDialect}} to
convert it back into something understood by the RDBMS (Oracle in that case).
> PushDownJoinConditions is not always a valid transformation
> ---
>
> Key: CALCITE-3101
> URL: https://issues.apache.org/jira/browse/CALCITE-3101
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.16.0
> Environment: Java app pointing to Oracle database.
>Reporter: Paul Jackson
>Priority: Major
>
> SqlToRelConverter can create a plan that is invalid when converted back to
> SQL in cases where the expression that is pushed to the projection returns a
> Boolean. The following example pushes IS NOT NULL to a select. Several SQL
> dialects do not support this. Oracle, for example, sees IS NOT NULL as a
> condition rather than an expression. It returns a Boolean data type, which is
> not supported. Likewise, Microsoft SQL Server does not support IS NOT NULL in
> a projection expression.
> Steps to reproduce (Oracle) :
> DDL:
> {{CREATE TABLE "EMP" (}}
> {{ "empno" INTEGER PRIMARY KEY NOT NULL,}}
> {{ "ename" VARCHAR(100),}}
> {{"deptno" INTEGER);}}
> Start with this query:
> {{SELECT "EMP"."empno", "t"."ename" "ename0"}}
> {{FROM "EMP"}}
> {{INNER JOIN "EMP" "t"}}
> {{ON "EMP"."deptno" = "t"."deptno" AND "t"."ename" IS NOT NULL}}
> Parse using SqlToRelConverter.convertQuery()
> At his point in the stack trace:
> {{org.apache.calcite.plan.RelOptUtil.pushDownJoinConditions(RelOptUtil.java:3222)}}
> {{org.apache.calcite.sql2rel.SqlToRelConverter.createJoin(SqlToRelConverter.java:2414)}}
> {{org.apache.calcite.sql2rel.SqlToRelConverter.convertFrom(SqlToRelConverter.java:2056)}}
> {{org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:641)}}
> {{org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:622)}}
> {{org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3057)}}
> {{org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:558)}}
> the RelNode is:
> {{LogicalJoin(condition=[AND(=($3, $8), IS NOT NULL($6))], joinType=[inner])}}
> {{ JdbcTableScan(table=[[XYZ, EMP]])}}
> {{ JdbcTableScan(table=[[XYZ, EMP]])}}
> After pushDownJoinConditions the RelNode is:
> {{LogicalJoin(condition=[AND(=($3, $8), $10)], joinType=[inner])}}
> {{ JdbcTableScan(table=[[XYZ, EMP]])}}
> {{ LogicalProject(empno=[$0], ename=[$1], job=[$2], deptno=[$3], etype=[$4],
> $f5=[IS NOT NULL($1)])}}
> {{ JdbcTableScan(table=[[XYZ, EMP]])}}
> Which leads to invalid SQL (ORA-00923: FROM keyword not found where expected)
> :
> {{SELECT "EMP"."empno", "t"."ename" "ename0"}}
> {{FROM "XYZ"."EMP"}}
> {{INNER JOIN (SELECT "empno", "ename", "job", "deptno", "etype", "ename" IS
> NOT NULL "$f5"}}
> {{FROM "XYZ"."EMP") "t" ON "EMP"."deptno" = "t"."deptno" AND "t"."$f5"}}
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3085) Unused stack field in RelShuttleImpl
[
https://issues.apache.org/jira/browse/CALCITE-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16850155#comment-16850155
]
Laurent Goujon commented on CALCITE-3085:
-
It's so useful no subclasses uses it (unlike modCound) :) TBH I don't have the
observation anymore as it was a change one of my team mate made some time
(year) ago, and didn't left much detail except for avoiding the extra
allocation, and also being able to create threadsafe instances of
RelShuttleImpl (and so being able to use singleton).
I also offered a second alternative (see my comment above) to create an
alternative implementation. If people are more comfortable with that second
approach:
- Would people be okay of changing current calcite shuttles to use the
alternative? And replacing instances of RelShuttleImpl with no state by
singleton instances?
- Would people be okay with renaming current RelShuttleImpl
StackingRelShuttleImpl and the alternative RelShuttleImpl
> Unused stack field in RelShuttleImpl
>
>
> Key: CALCITE-3085
> URL: https://issues.apache.org/jira/browse/CALCITE-3085
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {{RelShuttleImpl}} class has a protected {{stack}} field which is being
> populated when going over children, but those content is actually never used.
> In Calcite code base, no subclasses are actually using the content of the
> stack ({{CorelMapBuilder}} is populating the stack but does not read the
> content back either).
> Searching code on github didn't show any usage of it either (but this is not
> foolprof).
> As maintaining this stack has a non-negligible impact on memory/performance,
> I would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3085) Unused stack field in RelShuttleImpl
[
https://issues.apache.org/jira/browse/CALCITE-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846395#comment-16846395
]
Laurent Goujon commented on CALCITE-3085:
-
This is why I did check on Github. [~danny0405] are you aware of any class
using the stack?
> Unused stack field in RelShuttleImpl
>
>
> Key: CALCITE-3085
> URL: https://issues.apache.org/jira/browse/CALCITE-3085
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{RelShuttleImpl}} class has a protected {{stack}} field which is being
> populated when going over children, but those content is actually never used.
> In Calcite code base, no subclasses are actually using the content of the
> stack ({{CorelMapBuilder}} is populating the stack but does not read the
> content back either).
> Searching code on github didn't show any usage of it either (but this is not
> foolprof).
> As maintaining this stack has a non-negligible impact on memory/performance,
> I would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3085) Unused stack field in RelShuttleImpl
[
https://issues.apache.org/jira/browse/CALCITE-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846271#comment-16846271
]
Laurent Goujon commented on CALCITE-3085:
-
Alternative would be to create a variant of the class without the stack and
move Calcite classes over, but like I said, didn't find usage of that field
(and it seems we don't have a strong policy of never breaking compatibility,
although we tried to avoid to).
> Unused stack field in RelShuttleImpl
>
>
> Key: CALCITE-3085
> URL: https://issues.apache.org/jira/browse/CALCITE-3085
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>
> {{RelShuttleImpl}} class has a protected {{stack}} field which is being
> populated when going over children, but those content is actually never used.
> In Calcite code base, no subclasses are actually using the content of the
> stack ({{CorelMapBuilder}} is populating the stack but does not read the
> content back either).
> Searching code on github didn't show any usage of it either (but this is not
> foolprof).
> As maintaining this stack has a non-negligible impact on memory/performance,
> I would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (CALCITE-3085) Unused stack field in RelShuttleImpl
[
https://issues.apache.org/jira/browse/CALCITE-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-3085:
Priority: Minor (was: Major)
> Unused stack field in RelShuttleImpl
>
>
> Key: CALCITE-3085
> URL: https://issues.apache.org/jira/browse/CALCITE-3085
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Minor
>
> {{RelShuttleImpl}} class has a protected {{stack}} field which is being
> populated when going over children, but those content is actually never used.
> In Calcite code base, no subclasses are actually using the content of the
> stack ({{CorelMapBuilder}} is populating the stack but does not read the
> content back either).
> Searching code on github didn't show any usage of it either (but this is not
> foolprof).
> As maintaining this stack has a non-negligible impact on memory/performance,
> I would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (CALCITE-3085) Unused stack field in RelShuttleImpl
[
https://issues.apache.org/jira/browse/CALCITE-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-3085:
Issue Type: Improvement (was: Bug)
> Unused stack field in RelShuttleImpl
>
>
> Key: CALCITE-3085
> URL: https://issues.apache.org/jira/browse/CALCITE-3085
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>
> {{RelShuttleImpl}} class has a protected {{stack}} field which is being
> populated when going over children, but those content is actually never used.
> In Calcite code base, no subclasses are actually using the content of the
> stack ({{CorelMapBuilder}} is populating the stack but does not read the
> content back either).
> Searching code on github didn't show any usage of it either (but this is not
> foolprof).
> As maintaining this stack has a non-negligible impact on memory/performance,
> I would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-3085) Unused stack field in RelShuttleImpl
Laurent Goujon created CALCITE-3085:
---
Summary: Unused stack field in RelShuttleImpl
Key: CALCITE-3085
URL: https://issues.apache.org/jira/browse/CALCITE-3085
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Assignee: Laurent Goujon
{{RelShuttleImpl}} class has a protected {{stack}} field which is being
populated when going over children, but those content is actually never used.
In Calcite code base, no subclasses are actually using the content of the stack
({{CorelMapBuilder}} is populating the stack but does not read the content back
either).
Searching code on github didn't show any usage of it either (but this is not
foolprof).
As maintaining this stack has a non-negligible impact on memory/performance, I
would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (CALCITE-2807) Identify expanded IS NOT DISTINCT FROM expression when pushing down filter past join
[ https://issues.apache.org/jira/browse/CALCITE-2807?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Laurent Goujon resolved CALCITE-2807. - Resolution: Fixed Assignee: Laurent Goujon Fix Version/s: 1.20.0 > Identify expanded IS NOT DISTINCT FROM expression when pushing down filter > past join > > > Key: CALCITE-2807 > URL: https://issues.apache.org/jira/browse/CALCITE-2807 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Laurent Goujon >Assignee: Laurent Goujon >Priority: Major > Labels: pull-request-available > Fix For: 1.20.0 > > Time Spent: 20m > Remaining Estimate: 0h > > IS NOT DISTINCT FROM expressions in join condition might actually be > considered as equi-join conditions, and RelOptUtil#splitJoinConditions() has > support for it. But some other join related functions/rules don't. > One of them is RelOptUtil#pushDownJoinConditions (used by > JoinPushExpressionsRule) which tries to push filter expressions below the > join, but ends up modifying the join expression in a way which makes identify > an IDNF condition impossible later. > For example expression OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4)) will > be changed into OR(AND($3, $6), EQUALS($1, $5)) which makes it > harder/impossible for RelOptUtil#splitJoinConditions() to identify an IS NOT > DISTINCT FROM equi-join condition. > This is a variant of CALCITE-2803 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2807) Identify expanded IS NOT DISTINCT FROM expression when pushing down filter past join
[ https://issues.apache.org/jira/browse/CALCITE-2807?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845293#comment-16845293 ] Laurent Goujon commented on CALCITE-2807: - I was not necessary thinking of Github guidelines but guidelines outline in the git documentation (https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project in section Commit Guidelines), which is kind of captured by https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html. Looks like Linux Kernel development is happy with a summary line with more than 50 characters (but less than 70-75). Calcite guidelines just mention "The first line of the commit message must be a concise and useful description of the change." Not really surprise by Linus' comments regarding Github UI (I have my own beef with some of the UI choices too). > Identify expanded IS NOT DISTINCT FROM expression when pushing down filter > past join > > > Key: CALCITE-2807 > URL: https://issues.apache.org/jira/browse/CALCITE-2807 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Laurent Goujon >Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > IS NOT DISTINCT FROM expressions in join condition might actually be > considered as equi-join conditions, and RelOptUtil#splitJoinConditions() has > support for it. But some other join related functions/rules don't. > One of them is RelOptUtil#pushDownJoinConditions (used by > JoinPushExpressionsRule) which tries to push filter expressions below the > join, but ends up modifying the join expression in a way which makes identify > an IDNF condition impossible later. > For example expression OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4)) will > be changed into OR(AND($3, $6), EQUALS($1, $5)) which makes it > harder/impossible for RelOptUtil#splitJoinConditions() to identify an IS NOT > DISTINCT FROM equi-join condition. > This is a variant of CALCITE-2803 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2807) Identify expanded IS NOT DISTINCT FROM expression when pushing down filter past join
[ https://issues.apache.org/jira/browse/CALCITE-2807?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845286#comment-16845286 ] Laurent Goujon commented on CALCITE-2807: - I agree that IDNF is jargon but I actually mentioned IS NOT DISTINCT FROM in the commit message, as the first occurrence (except for the summary where it would make the line goes past 50 characters, which is above general git commit format guidelines). Do you want me to expand all the occurrences of INDF? > Identify expanded IS NOT DISTINCT FROM expression when pushing down filter > past join > > > Key: CALCITE-2807 > URL: https://issues.apache.org/jira/browse/CALCITE-2807 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Laurent Goujon >Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > IS NOT DISTINCT FROM expressions in join condition might actually be > considered as equi-join conditions, and RelOptUtil#splitJoinConditions() has > support for it. But some other join related functions/rules don't. > One of them is RelOptUtil#pushDownJoinConditions (used by > JoinPushExpressionsRule) which tries to push filter expressions below the > join, but ends up modifying the join expression in a way which makes identify > an IDNF condition impossible later. > For example expression OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4)) will > be changed into OR(AND($3, $6), EQUALS($1, $5)) which makes it > harder/impossible for RelOptUtil#splitJoinConditions() to identify an IS NOT > DISTINCT FROM equi-join condition. > This is a variant of CALCITE-2803 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (CALCITE-3082) NPE in SqlUtil#getSelectListItem
[
https://issues.apache.org/jira/browse/CALCITE-3082?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-3082:
Description:
Queries similar to {{SELECT 1 UNION SELECT 2, 3}} causes validator to throws a
{{NullPointerException}} instance instead of a proper error message about the
column count mismatch.
{{SqlUtil#getSelectListItem}} is used by Set operators to get select items if
the types of operands don't match. Unfortunately the method may throw an NPE
exception if one of the operand doesn't have a FROM clause.
was:{{SqlUtil#getSelectListItem}} is used by Set operators to get select
items if the types of operands don't match. Unfortunately the method may throw
an NPE exception if one of the operand doesn't have a FROM clause.
> NPE in SqlUtil#getSelectListItem
>
>
> Key: CALCITE-3082
> URL: https://issues.apache.org/jira/browse/CALCITE-3082
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Queries similar to {{SELECT 1 UNION SELECT 2, 3}} causes validator to throws
> a {{NullPointerException}} instance instead of a proper error message about
> the column count mismatch.
> {{SqlUtil#getSelectListItem}} is used by Set operators to get select items if
> the types of operands don't match. Unfortunately the method may throw an NPE
> exception if one of the operand doesn't have a FROM clause.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-3082) NPE in SqlUtil#getSelectListItem
Laurent Goujon created CALCITE-3082:
---
Summary: NPE in SqlUtil#getSelectListItem
Key: CALCITE-3082
URL: https://issues.apache.org/jira/browse/CALCITE-3082
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Assignee: Laurent Goujon
{{SqlUtil#getSelectListItem}} is used by Set operators to get select items if
the types of operands don't match. Unfortunately the method may throw an NPE
exception if one of the operand doesn't have a FROM clause.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2421) RexSimplify#simplifyAnds foregoes some simplications if unknownAsFalse set to true
[
https://issues.apache.org/jira/browse/CALCITE-2421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842764#comment-16842764
]
Laurent Goujon commented on CALCITE-2421:
-
[~kgyrtkirk] sorry for not being responsive. No problem, I'll look at your PR
(if still okay)
> RexSimplify#simplifyAnds foregoes some simplications if unknownAsFalse set to
> true
> --
>
> Key: CALCITE-2421
> URL: https://issues.apache.org/jira/browse/CALCITE-2421
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> It looks like {{RexSimplify#simplifyAnds}} foregoes some comparison
> simplifications if {{unknownAsFalse}} is set to true, like {{A = A AND B =
> B}} which might be simplified to {{A IS NOT NULL AND B IS NOT NULL}} or even
> {{true}} if {{A}} and {{B}} are known to be not nullable.
> One consequence of this is that the selectivity value might be off as a {{=}}
> comparison has a selectivity of 15% whereas {{IS NOT NULL}} has a selectivity
> of 90%.
> The simplication is skipped because {{RexSimplify#simplifyList}} simplify all
> terms with {{unknownAsFalse}} set to {{false}}, but in
> {{RexSimplify#simplifyAnd2ForUnknownAsFalse}}, there's no attempt at trying
> again to simplify each term.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2397) Column Index not getting set on correlationIds
[ https://issues.apache.org/jira/browse/CALCITE-2397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842498#comment-16842498 ] Laurent Goujon commented on CALCITE-2397: - [~julianhyde] [~vladimirsitnikov] we missed some of the changes done to correlationid during calcite development and came up with that unnecessary approach. Just verified that the change is not necessary anymore. > Column Index not getting set on correlationIds > -- > > Key: CALCITE-2397 > URL: https://issues.apache.org/jira/browse/CALCITE-2397 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.16.0 >Reporter: James Duong >Priority: Major > Labels: pull-request-available > > During SqlToRel conversion, the column index bitset is not getting set on > CorrelationIds. This causes getCorrelationId() to always return empty. > Also get/setColumnIndex() have been removed from CorrelationId -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (CALCITE-3062) Do not populate provenanceMap if not used
[
https://issues.apache.org/jira/browse/CALCITE-3062?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-3062.
-
Resolution: Fixed
Fix Version/s: 1.20.0
Merged as
https://github.com/apache/calcite/commit/c6b6800c220e513f1d9a2b167b2f14cb689c0b06
> Do not populate provenanceMap if not used
> -
>
> Key: CALCITE-3062
> URL: https://issues.apache.org/jira/browse/CALCITE-3062
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> {{VolcanoPlanner}}'s field {{provenanceMap}} tracks the provenance of each
> node being added to the planner, but the information is only used when the
> log level of the planner of the debug, so when planner is not running in
> debug mode, this data goes to waste and used memory unnecessary.
> The planner should be changed so that the information is only captured if
> used later.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (CALCITE-2803) Identify expanded IS NOT DISTINCT FROM expression when pushing project past join
[
https://issues.apache.org/jira/browse/CALCITE-2803?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-2803.
-
Resolution: Fixed
> Identify expanded IS NOT DISTINCT FROM expression when pushing project past
> join
>
>
> Key: CALCITE-2803
> URL: https://issues.apache.org/jira/browse/CALCITE-2803
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> {{IS NOT DISTINCT FROM}} expressions in join condition might actually be
> considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
> has support for it. But some other join related functions/rules don't.
> One of them is {{ProjectJoinTransposeRule}} which tries to push project
> expressions below the join, but ends up modifying the join expression too by
> pushing complex expression below.
> For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
> will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
> harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an
> {{IS NOT DISTINCT FROM}} equi-join condition.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (CALCITE-2803) Identify expanded IS NOT DISTINCT FROM expression when pushing project past join
[
https://issues.apache.org/jira/browse/CALCITE-2803?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-2803:
Fix Version/s: 1.20.0
> Identify expanded IS NOT DISTINCT FROM expression when pushing project past
> join
>
>
> Key: CALCITE-2803
> URL: https://issues.apache.org/jira/browse/CALCITE-2803
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> {{IS NOT DISTINCT FROM}} expressions in join condition might actually be
> considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
> has support for it. But some other join related functions/rules don't.
> One of them is {{ProjectJoinTransposeRule}} which tries to push project
> expressions below the join, but ends up modifying the join expression too by
> pushing complex expression below.
> For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
> will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
> harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an
> {{IS NOT DISTINCT FROM}} equi-join condition.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Assigned] (CALCITE-2803) Identify expanded IS NOT DISTINCT FROM expression when pushing project past join
[
https://issues.apache.org/jira/browse/CALCITE-2803?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon reassigned CALCITE-2803:
---
Assignee: Laurent Goujon
> Identify expanded IS NOT DISTINCT FROM expression when pushing project past
> join
>
>
> Key: CALCITE-2803
> URL: https://issues.apache.org/jira/browse/CALCITE-2803
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> {{IS NOT DISTINCT FROM}} expressions in join condition might actually be
> considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
> has support for it. But some other join related functions/rules don't.
> One of them is {{ProjectJoinTransposeRule}} which tries to push project
> expressions below the join, but ends up modifying the join expression too by
> pushing complex expression below.
> For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
> will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
> harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an
> {{IS NOT DISTINCT FROM}} equi-join condition.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3062) Do not populate provenanceMap if not used
[
https://issues.apache.org/jira/browse/CALCITE-3062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838652#comment-16838652
]
Laurent Goujon commented on CALCITE-3062:
-
To my knowledge, I don't recollect looking at the provenanceMap directly while
debugging a test. The {{DEBUG}} log would actually gives much more useful
output. I guess it could be possible to check if assertions are turned on and
also enable the capture too?
> Do not populate provenanceMap if not used
> -
>
> Key: CALCITE-3062
> URL: https://issues.apache.org/jira/browse/CALCITE-3062
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{VolcanoPlanner}}'s field {{provenanceMap}} tracks the provenance of each
> node being added to the planner, but the information is only used when the
> log level of the planner of the debug, so when planner is not running in
> debug mode, this data goes to waste and used memory unnecessary.
> The planner should be changed so that the information is only captured if
> used later.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3062) Do not populate provenanceMap if not used
[
https://issues.apache.org/jira/browse/CALCITE-3062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16837894#comment-16837894
]
Laurent Goujon commented on CALCITE-3062:
-
Can you clarify what do you mean by that? There's only one logger and one level
involved which is {{CalciteTrace.getPlannerTracer()}}/{{DEBUG}} and the
behavior is pretty clear. The change I suggest is to not populate the map if
the log level is at least not {{DEBUG}}.
> Do not populate provenanceMap if not used
> -
>
> Key: CALCITE-3062
> URL: https://issues.apache.org/jira/browse/CALCITE-3062
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{VolcanoPlanner}}'s field {{provenanceMap}} tracks the provenance of each
> node being added to the planner, but the information is only used when the
> log level of the planner of the debug, so when planner is not running in
> debug mode, this data goes to waste and used memory unnecessary.
> The planner should be changed so that the information is only captured if
> used later.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3062) Do not populate provenanceMap if not used
[
https://issues.apache.org/jira/browse/CALCITE-3062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16837762#comment-16837762
]
Laurent Goujon commented on CALCITE-3062:
-
I didn't observe the behavior you mention. Based on my research, only two
classes access the field (VolcanoPlanner and RelSubset) and only VolcanoPlanner
actually reads the content of the map, at the end of {{findBestExp}} method
call, only in debug mode, to display the provenance of each node of the best
plan... Changing the behavior of the planner to not populate the map doesn't
impact any of the Calcite tests too.
> Do not populate provenanceMap if not used
> -
>
> Key: CALCITE-3062
> URL: https://issues.apache.org/jira/browse/CALCITE-3062
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{VolcanoPlanner}}'s field {{provenanceMap}} tracks the provenance of each
> node being added to the planner, but the information is only used when the
> log level of the planner of the debug, so when planner is not running in
> debug mode, this data goes to waste and used memory unnecessary.
> The planner should be changed so that the information is only captured if
> used later.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-3062) Do not populate provenanceMap if not used
Laurent Goujon created CALCITE-3062:
---
Summary: Do not populate provenanceMap if not used
Key: CALCITE-3062
URL: https://issues.apache.org/jira/browse/CALCITE-3062
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Laurent Goujon
Assignee: Laurent Goujon
{{VolcanoPlanner}}'s field {{provenanceMap}} tracks the provenance of each node
being added to the planner, but the information is only used when the log level
of the planner of the debug, so when planner is not running in debug mode, this
data goes to waste and used memory unnecessary.
The planner should be changed so that the information is only captured if used
later.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (CALCITE-2340) Invalid cast generated by AvgVarianceConvertlet
[
https://issues.apache.org/jira/browse/CALCITE-2340?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-2340.
-
Resolution: Fixed
Fix Version/s: 1.18.0
> Invalid cast generated by AvgVarianceConvertlet
> ---
>
> Key: CALCITE-2340
> URL: https://issues.apache.org/jira/browse/CALCITE-2340
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.18.0
>
>
> A bug in {{AvgVarianceConvertlet}} causes {{CAST}} calls to be generated with
> the wrong number of arguments if the return type of the aggregation function
> is different from the operand.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2340) Invalid cast generated by AvgVarianceConvertlet
[
https://issues.apache.org/jira/browse/CALCITE-2340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16785024#comment-16785024
]
Laurent Goujon commented on CALCITE-2340:
-
Actually fixed as part of the work for CALCITE-2402 (apache/calcite#779)
> Invalid cast generated by AvgVarianceConvertlet
> ---
>
> Key: CALCITE-2340
> URL: https://issues.apache.org/jira/browse/CALCITE-2340
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
>
> A bug in {{AvgVarianceConvertlet}} causes {{CAST}} calls to be generated with
> the wrong number of arguments if the return type of the aggregation function
> is different from the operand.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (CALCITE-2830) Fix AvgVarianceConvertlet
[ https://issues.apache.org/jira/browse/CALCITE-2830?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Laurent Goujon resolved CALCITE-2830. - Resolution: Duplicate > Fix AvgVarianceConvertlet > - > > Key: CALCITE-2830 > URL: https://issues.apache.org/jira/browse/CALCITE-2830 > Project: Calcite > Issue Type: Bug >Reporter: Siddharth Teotia >Priority: Major > > Fix a bug in AvgVarianceConvertlet where an invalid CAST call with an invalid > number of arguments (1 instead of 2) would be generated if the infered > returned type for the call is not the same as the type from the original > operand. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2340) Invalid cast generated by AvgVarianceConvertlet
[
https://issues.apache.org/jira/browse/CALCITE-2340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16784993#comment-16784993
]
Laurent Goujon commented on CALCITE-2340:
-
Let me do that
> Invalid cast generated by AvgVarianceConvertlet
> ---
>
> Key: CALCITE-2340
> URL: https://issues.apache.org/jira/browse/CALCITE-2340
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Labels: pull-request-available
>
> A bug in {{AvgVarianceConvertlet}} causes {{CAST}} calls to be generated with
> the wrong number of arguments if the return type of the aggregation function
> is different from the operand.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2142) Set rand() function as non-deterministic.
[ https://issues.apache.org/jira/browse/CALCITE-2142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16781156#comment-16781156 ] Laurent Goujon commented on CALCITE-2142: - Moreover RAND(1) is deterministic row wise, but not query wise (the function returns different values for each row), and I'm not sure how it should be captured. The operator is marked as dynamic, but that is also quite not right, because one might be okay to do constant reduction for dynamic functions if one knows it is not something which will be cached, but RAND() should not be constant reduced. And most of the simplication code only looks at the deterministic flag (RexUtil.isDeterministic()) but not at the dynamic one. > Set rand() function as non-deterministic. > - > > Key: CALCITE-2142 > URL: https://issues.apache.org/jira/browse/CALCITE-2142 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Josh Wagner >Priority: Minor > > SqlRandFunction should override isDeterministic() such that it returns false. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2142) Set rand() function as non-deterministic.
[
https://issues.apache.org/jira/browse/CALCITE-2142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16781152#comment-16781152
]
Laurent Goujon commented on CALCITE-2142:
-
Sorry for the long delay.
First, I think there is some confusion about {{SqlRandFunction}} which is the
operator from {{RandFunction}} which is the generator.
The fix would apply to the operator. As for the function itself, it is really
confusing because the annotation says:
{quote}
* Specifies that function is deterministic (i.e. returns the same output
* given the same inputs).
{quote}
But there's comment in {{RandFunction}} saying:
{quote}
Marked deterministic so that the code generator instantiates one once per
query, not once per row
{quote}
Looking at {{misc.iq}} as you suggested make me think that the issue is calcite
needs the generator to be executed only once, probably because of the seed (you
don't want to reset the seed between rows) which is why you added the
annotation, but overall the function is non deterministic.
Or maybe it depends if there's a seed or not? like {{RAND() == RAND()) is FALSE
(most likely) but {{RAND(1) == RAND(1)}} is TRUE (always)?
> Set rand() function as non-deterministic.
> -
>
> Key: CALCITE-2142
> URL: https://issues.apache.org/jira/browse/CALCITE-2142
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Josh Wagner
>Priority: Minor
>
> SqlRandFunction should override isDeterministic() such that it returns false.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2823) Prevent optimization of non-deterministic functions
[ https://issues.apache.org/jira/browse/CALCITE-2823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16781102#comment-16781102 ] Laurent Goujon commented on CALCITE-2823: - Is RAND() deterministic? no, because it returns a different value each time the function is called. But there are several places in Calcite when RexNodeA == RexNodeB is tested, and that equality is of the result is assumed because expressions are equals. The same issue is also present in linq4j, unfortunately, I'm not really knowledgeable about that part of the code, and not sure how to address it. > Prevent optimization of non-deterministic functions > --- > > Key: CALCITE-2823 > URL: https://issues.apache.org/jira/browse/CALCITE-2823 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Siddharth Teotia >Priority: Major > > RexSimplify and RexProgram optimize expressions like A() = A() to true, but > if A() is a non-deterministic expression, this optimization is invalid. > Change RexSimplify and RexProgram to check if expression is deterministic or > not, in order to decide about the optimization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2827) Allow CONVENTION.NONE planning with VolcanoPlanner
[ https://issues.apache.org/jira/browse/CALCITE-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16779974#comment-16779974 ] Laurent Goujon commented on CALCITE-2827: - Didn't I answer it? Our planner has a planning phase when there's no convention change, but we are looking for a cost-based solution when applying different rules. Because NONE convention has always infinite cost, by default VolcanoPlanner cannot answer it. Note that if nodes were using a different convention, it would not be issue. I interpret this as VolcanoPlanner has an optimization to not compute cost for nodes with NONE convention since in most cases we are interested in finding the best solution in a different convention. But maybe that optimization could be generalized, and the optimization would be to not compute cost for the source convention, and make the optimization optional. If you think I merged the change too soon (but from [~michaelmior] PR comment, I didn't think it was too much controversial), I don't mind reverting. > Allow CONVENTION.NONE planning with VolcanoPlanner > -- > > Key: CALCITE-2827 > URL: https://issues.apache.org/jira/browse/CALCITE-2827 > Project: Calcite > Issue Type: Task >Reporter: Siddharth Teotia >Assignee: Juhwan Kim >Priority: Minor > Labels: pull-request-available > Fix For: 1.19.0 > > Time Spent: 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (CALCITE-2827) Allow CONVENTION.NONE planning with VolcanoPlanner
[ https://issues.apache.org/jira/browse/CALCITE-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Laurent Goujon resolved CALCITE-2827. - Resolution: Fixed Fix Version/s: 1.19.0 > Allow CONVENTION.NONE planning with VolcanoPlanner > -- > > Key: CALCITE-2827 > URL: https://issues.apache.org/jira/browse/CALCITE-2827 > Project: Calcite > Issue Type: Task >Reporter: Siddharth Teotia >Assignee: Juhwan Kim >Priority: Minor > Labels: pull-request-available > Fix For: 1.19.0 > > Time Spent: 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Assigned] (CALCITE-2827) Allow CONVENTION.NONE planning with VolcanoPlanner
[ https://issues.apache.org/jira/browse/CALCITE-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Laurent Goujon reassigned CALCITE-2827: --- Assignee: Juhwan Kim > Allow CONVENTION.NONE planning with VolcanoPlanner > -- > > Key: CALCITE-2827 > URL: https://issues.apache.org/jira/browse/CALCITE-2827 > Project: Calcite > Issue Type: Task >Reporter: Siddharth Teotia >Assignee: Juhwan Kim >Priority: Minor > Labels: pull-request-available > Time Spent: 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2827) Allow CONVENTION.NONE planning with VolcanoPlanner
[ https://issues.apache.org/jira/browse/CALCITE-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16779769#comment-16779769 ] Laurent Goujon commented on CALCITE-2827: - Use case is to use volcano planner for doing cost based optimization without changing convention. > Allow CONVENTION.NONE planning with VolcanoPlanner > -- > > Key: CALCITE-2827 > URL: https://issues.apache.org/jira/browse/CALCITE-2827 > Project: Calcite > Issue Type: Task >Reporter: Siddharth Teotia >Priority: Minor > Labels: pull-request-available > Time Spent: 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (CALCITE-2807) Identify expanded IS NOT DISTINCT FROM expression when pushing down filter past join
Laurent Goujon created CALCITE-2807: --- Summary: Identify expanded IS NOT DISTINCT FROM expression when pushing down filter past join Key: CALCITE-2807 URL: https://issues.apache.org/jira/browse/CALCITE-2807 Project: Calcite Issue Type: Improvement Components: core Reporter: Laurent Goujon Assignee: Julian Hyde IS NOT DISTINCT FROM expressions in join condition might actually be considered as equi-join conditions, and RelOptUtil#splitJoinConditions() has support for it. But some other join related functions/rules don't. One of them is RelOptUtil#pushDownJoinConditions (used by JoinPushExpressionsRule) which tries to push filter expressions below the join, but ends up modifying the join expression in a way which makes identify an IDNF condition impossible later. For example expression OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4)) will be changed into OR(AND($3, $6), EQUALS($1, $5)) which makes it harder/impossible for RelOptUtil#splitJoinConditions() to identify an IS NOT DISTINCT FROM equi-join condition. This is a variant of CALCITE-2803 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2803) Identify expanded IS NOT DISTINCT FROM expression when pushing project past join
[
https://issues.apache.org/jira/browse/CALCITE-2803?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16752591#comment-16752591
]
Laurent Goujon commented on CALCITE-2803:
-
One possible solution is to normalize the join condition just before using
{{PushProjector}} so that the expanded {{IS NOT DISTINCT FROM}} expressions are
converted back into the collapsed version, and so doesn't cause extra
expressions to be created.
Another solution is not to modify filter condition since this is also managed
by {{JoinPushExpressionsRule}} anyway.
> Identify expanded IS NOT DISTINCT FROM expression when pushing project past
> join
>
>
> Key: CALCITE-2803
> URL: https://issues.apache.org/jira/browse/CALCITE-2803
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> {{IS NOT DISTINCT FROM}} expressions in join condition might actually be
> considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
> has support for it. But some other join related functions/rules don't.
> One of them is {{ProjectJoinTransposeRule}} which tries to push project
> expressions below the join, but ends up modifying the join expression too by
> pushing complex expression below.
> For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
> will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
> harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an
> {{IS NOT DISTINCT FROM}} equi-join condition.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (CALCITE-2803) Identify expanded IS NOT DISTINCT FROM expression when pushing project past join
[
https://issues.apache.org/jira/browse/CALCITE-2803?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon updated CALCITE-2803:
Description:
{{IS NOT DISTINCT FROM}} expressions in join condition might actually be
considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
has support for it. But some other join related functions/rules don't.
One of them is {{ProjectJoinTransposeRule}} which tries to push project
expressions below the join, but ends up modifying the join expression too by
pushing complex expression below.
For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an {{IS
NOT DISTINCT FROM}} equi-join condition.
was:
{{IS NOT DISTINCT FROM}} expressions in join condition might actually be
considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
has support for it. But some other join related functions/rules don't.
One of them is {{ProjectJoinTransposeRule}} which tries to push project
expressions below the join, but ends up modifying the join expression too by
pushing complex expression below/
For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an {{IS
NOT DISTINCT FROM}} equi-join condition.
> Identify expanded IS NOT DISTINCT FROM expression when pushing project past
> join
>
>
> Key: CALCITE-2803
> URL: https://issues.apache.org/jira/browse/CALCITE-2803
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> {{IS NOT DISTINCT FROM}} expressions in join condition might actually be
> considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
> has support for it. But some other join related functions/rules don't.
> One of them is {{ProjectJoinTransposeRule}} which tries to push project
> expressions below the join, but ends up modifying the join expression too by
> pushing complex expression below.
> For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
> will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
> harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an
> {{IS NOT DISTINCT FROM}} equi-join condition.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-2803) Identify expanded IS NOT DISTINCT FROM expression when pushing project past join
Laurent Goujon created CALCITE-2803:
---
Summary: Identify expanded IS NOT DISTINCT FROM expression when
pushing project past join
Key: CALCITE-2803
URL: https://issues.apache.org/jira/browse/CALCITE-2803
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Laurent Goujon
Assignee: Julian Hyde
{{IS NOT DISTINCT FROM}} expressions in join condition might actually be
considered as equi-join conditions, and {{RelOptUtil#splitJoinConditions()}}
has support for it. But some other join related functions/rules don't.
One of them is {{ProjectJoinTransposeRule}} which tries to push project
expressions below the join, but ends up modifying the join expression too by
pushing complex expression below/
For example expression {{OR(AND(IS_NULL($1), IS_NULL($4)), EQUALS($1,$4))}}
will be changed into {{OR(AND($3, $6), EQUALS($1, $5))}} which makes it
harder/impossible for {{RelOptUtil#splitJoinConditions()}} to identify an {{IS
NOT DISTINCT FROM}} equi-join condition.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2437) FilterMultiJoinMergeRule doesn't combine postFilterCondition
[
https://issues.apache.org/jira/browse/CALCITE-2437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726940#comment-16726940
]
Laurent Goujon commented on CALCITE-2437:
-
I guessed I just jinxed myself? I actually triggered it somehow. Will submit a
patch shortly
> FilterMultiJoinMergeRule doesn't combine postFilterCondition
>
>
> Key: CALCITE-2437
> URL: https://issues.apache.org/jira/browse/CALCITE-2437
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> {{FilterMultiJoinMergeRule}} merges a {{Filter}} on top of a {{MultiJoin}}
> into the multi join itself as part of its post-join filter condition. But the
> rule doesn't seem to combine the top filter with the existing condition.
> I actually believe it's a bug in the code, although I never triggered it
> myself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2737) Depending on the version of Maven installed, the build will fail
[
https://issues.apache.org/jira/browse/CALCITE-2737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16718493#comment-16718493
]
Laurent Goujon commented on CALCITE-2737:
-
I tried with 3.3.9, 3.5.0 and 3.5.3, and was not able to reproduce the issue.
> Depending on the version of Maven installed, the build will fail
>
>
> Key: CALCITE-2737
> URL: https://issues.apache.org/jira/browse/CALCITE-2737
> Project: Calcite
> Issue Type: Bug
>Reporter: Jacques Nadeau
>Assignee: Jacques Nadeau
>Priority: Major
>
> On older versions of Maven, the maven compiler plugin needs to be informed
> that the system is using source version 1.8.
> {noformat}
> mvn -version
> Apache Maven 3.5.0 (ff8f5e7444045639af65f6095c62210b5713f426;
> 2017-04-04T01:09:06+05:30)
> Maven home: /usr/local/Cellar/maven/3.5.0/libexec
> Java version: 1.8.0_131, vendor: Oracle Corporation
> Java home:
> /Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/jre
> Default locale: en_US, platform encoding: UTF-8
> OS name: "mac os x", version: "10.13.6", arch: "x86_64", family:
> "mac"{noformat}
>
> {noformat}
> [INFO] -
> [ERROR] COMPILATION ERROR :
> [INFO] -
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[56,31]
> cannot find symbol
> symbol: method length()
> location: variable e1 of type java.lang.Object
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[57,28]
> cannot find symbol
> symbol: method charAt(int)
> location: variable e1 of type java.lang.Object
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[202,9]
> cannot infer type arguments for org.apache.calcite.util.PartiallyOrderedSet<>
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[218,9]
> cannot infer type arguments for org.apache.calcite.util.PartiallyOrderedSet<>
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[226,9]
> cannot infer type arguments for org.apache.calcite.util.PartiallyOrderedSet<>
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[273,9]
> cannot infer type arguments for org.apache.calcite.util.PartiallyOrderedSet<>
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[285,9]
> cannot infer type arguments for org.apache.calcite.util.PartiallyOrderedSet<>
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[301,16]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to int
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[308,18]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to int
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[321,18]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to int
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[332,16]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to java.lang.Integer
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[339,18]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to java.lang.Integer
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[352,18]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to java.lang.Integer
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[363,16]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to int
> [ERROR]
> /src/calcite/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java:[369,18]
> incompatible types: invalid method reference
> incompatible types: java.lang.Object cannot be converted to int
> [INFO] 15 errors
> [INFO] -
> [INFO]
>
> [INFO] BUILD FAILURE
> [INFO]
> {noformat}
--
This message was sent by Atlassian JIRA
[jira] [Commented] (CALCITE-2629) Unnecessary call to CatalogReader#getAllSchemaObjects in CatalogScope
[ https://issues.apache.org/jira/browse/CALCITE-2629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16661019#comment-16661019 ] Laurent Goujon commented on CALCITE-2629: - It looks like several tests in SqlAdvisorTest/SqlValidatorTest/SqlToRelConverterTest/SqlToRelConverterExtendedTest were exercising this code path, and are still present after 0938c7b6d7. Looks like the logic was refactored in DelegatingScope > Unnecessary call to CatalogReader#getAllSchemaObjects in CatalogScope > - > > Key: CALCITE-2629 > URL: https://issues.apache.org/jira/browse/CALCITE-2629 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Laurent Goujon >Assignee: Laurent Goujon >Priority: Minor > > CatalogScope constructor does a call to CatalogReader#getAllSchemaObjects and > store its result into a private field which has no accessor. This call is > actually expensive in our system, and is causing some performance issue > during planning -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2629) Unnecessary call to CatalogReader#getAllSchemaObjects in CatalogScope
[ https://issues.apache.org/jira/browse/CALCITE-2629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16654404#comment-16654404 ] Laurent Goujon commented on CALCITE-2629: - let me give it a try. Meanwhile, I posted a PR: https://github.com/apache/calcite/pull/888 > Unnecessary call to CatalogReader#getAllSchemaObjects in CatalogScope > - > > Key: CALCITE-2629 > URL: https://issues.apache.org/jira/browse/CALCITE-2629 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Laurent Goujon >Assignee: Laurent Goujon >Priority: Minor > > CatalogScope constructor does a call to CatalogReader#getAllSchemaObjects and > store its result into a private field which has no accessor. This call is > actually expensive in our system, and is causing some performance issue > during planning -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (CALCITE-2629) Unnecessary call to CatalogReader#getAllSchemaObjects in CatalogScope
Laurent Goujon created CALCITE-2629: --- Summary: Unnecessary call to CatalogReader#getAllSchemaObjects in CatalogScope Key: CALCITE-2629 URL: https://issues.apache.org/jira/browse/CALCITE-2629 Project: Calcite Issue Type: Bug Components: core Reporter: Laurent Goujon Assignee: Laurent Goujon CatalogScope constructor does a call to CatalogReader#getAllSchemaObjects and store its result into a private field which has no accessor. This call is actually expensive in our system, and is causing some performance issue during planning -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16606117#comment-16606117
]
Laurent Goujon commented on CALCITE-2537:
-
Attaching pull request (since stopped being done automatically for me):
https://github.com/apache/calcite/pull/820
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Assigned] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon reassigned CALCITE-2537:
---
Assignee: Laurent Goujon (was: Julian Hyde)
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605093#comment-16605093
]
Laurent Goujon commented on CALCITE-2537:
-
I would also be happy transforming the assertions into logging messages, since
obviously nothing really breaks (it might just cause a suboptimal plan)
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605091#comment-16605091
]
Laurent Goujon edited comment on CALCITE-2537 at 9/6/18 12:30 AM:
--
My point is more that log level should control verbosity and not have side
effects. Throwing an exception being a side-effect, that's why I'm suggesting
to add a check to see if assertions are enabled too.
was (Author: laurentgo):
My point is more that log level should control verbosity and not have side
effects. Throwing an exception being a side-effect, that's what I'm suggesting
to add a check to see if assertions are enabled too.
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605091#comment-16605091
]
Laurent Goujon commented on CALCITE-2537:
-
My point is more that log level should control verbosity and not have side
effects. Throwing an exception being a side-effect, that's what I'm suggesting
to add a check to see if assertions are enabled too.
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (CALCITE-2538) Missing RelMetadataQuery invalidation before subset cost improvement propagation
[
https://issues.apache.org/jira/browse/CALCITE-2538?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Laurent Goujon resolved CALCITE-2538.
-
Resolution: Duplicate
> Missing RelMetadataQuery invalidation before subset cost improvement
> propagation
>
>
> Key: CALCITE-2538
> URL: https://issues.apache.org/jira/browse/CALCITE-2538
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> When planner log level is set to debug, several tests are failing with the
> following error message:
> {noformat}
> rel
> [rel#11408:EnumerableProject.ENUMERABLE.[](input=rel#11180:Subset#2.ENUMERABLE.[0],DUMMY=0)]
> has lower cost {24.5 rows, 39.5 cpu, 0.0 io} than best cost {28.0 rows, 36.0
> cpu, 0.0 io} of subset [rel#11177:Subset#3.ENUMERABLE.[]]
> {noformat}
> The following query (from 'sql/agg.iq') should exhibit the issue:
> {code:sql}
> select sum(e.sal) as s from "scott".emp e join "scott".emp m on e.mgr =
> e.empno;
> {code}
> I tried to trace it back and my understanding is that during cost improvement
> propagation, a subset has its cost/best node changed, and it triggers
> recomputing the cost of the parent subset, but the current best node of the
> parent has its value cached in the current rel metadata query, and is not
> recomputed. During a later validation, this error is detected and the
> assertion is thrown.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2538) Missing RelMetadataQuery invalidation before subset cost improvement propagation
[
https://issues.apache.org/jira/browse/CALCITE-2538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605071#comment-16605071
]
Laurent Goujon commented on CALCITE-2538:
-
Seems very similar to CALCITE-2018
> Missing RelMetadataQuery invalidation before subset cost improvement
> propagation
>
>
> Key: CALCITE-2538
> URL: https://issues.apache.org/jira/browse/CALCITE-2538
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> When planner log level is set to debug, several tests are failing with the
> following error message:
> {noformat}
> rel
> [rel#11408:EnumerableProject.ENUMERABLE.[](input=rel#11180:Subset#2.ENUMERABLE.[0],DUMMY=0)]
> has lower cost {24.5 rows, 39.5 cpu, 0.0 io} than best cost {28.0 rows, 36.0
> cpu, 0.0 io} of subset [rel#11177:Subset#3.ENUMERABLE.[]]
> {noformat}
> The following query (from 'sql/agg.iq') should exhibit the issue:
> {code:sql}
> select sum(e.sal) as s from "scott".emp e join "scott".emp m on e.mgr =
> e.empno;
> {code}
> I tried to trace it back and my understanding is that during cost improvement
> propagation, a subset has its cost/best node changed, and it triggers
> recomputing the cost of the parent subset, but the current best node of the
> parent has its value cached in the current rel metadata query, and is not
> recomputed. During a later validation, this error is detected and the
> assertion is thrown.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605069#comment-16605069
]
Laurent Goujon edited comment on CALCITE-2537 at 9/6/18 12:05 AM:
--
It's not just the logging but the fact that {{validate}} checks the whole
planner tree (from sets to subsets to then validating cost of each node in the
subset) which take most of the time AND also throw an {{AssertionError}}
(because this is not supposed to happen), so this is more than just some extra
logging. I would prefer that checking log level is reserved when actually
printing out log statements (which is not the case here).
When working with external users to diagnose issue, asking them to turn all the
logs to debug have caused us to have a big slowdown and also from time to time
to get a different error from what we were currently trying to debug (see my
other ticket CALCITE-2538 as an example).
{{Objects.requireNonNull}} is more of a runtime check/preconditions than an
assertion so I wouldn't classify it the same way. My suggestion is to add both
the logger check and the assertion check to guard validate (It would actually
be nice to have it enabled for tests whatever the log level, but the
performance hit is really big).
was (Author: laurentgo):
It's not just the logging but the fact that validate checks the whole planner
tree (from sets to subsets to then validating cost of each node in the subset)
which take most of the time AND also throw an AssertionError (because this is
not supposed to happen), so this is more than just some extra logging.
When working with external users to diagnose issue, asking them to turn all the
logs to debug have caused us to have a big slowdown and also from time to time
to get a different error from what we were currently trying to debug (see my
other ticket CALCITE-2538 as an example).
{{Objects.requireNonNull}} is more of a runtime check/preconditions than an
assertion so I wouldn't classify it the same way. My suggestion is to add both
the logger check and the assertion check to guard validate (It would actually
be nice to have it enabled for tests whatever the log level, but the
performance hit is really big).
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
[
https://issues.apache.org/jira/browse/CALCITE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605069#comment-16605069
]
Laurent Goujon commented on CALCITE-2537:
-
It's not just the logging but the fact that validate checks the whole planner
tree (from sets to subsets to then validating cost of each node in the subset)
which take most of the time AND also throw an AssertionError (because this is
not supposed to happen), so this is more than just some extra logging.
When working with external users to diagnose issue, asking them to turn all the
logs to debug have caused us to have a big slowdown and also from time to time
to get a different error from what we were currently trying to debug (see my
other ticket CALCITE-2538 as an example).
{{Objects.requireNonNull}} is more of a runtime check/preconditions than an
assertion so I wouldn't classify it the same way. My suggestion is to add both
the logger check and the assertion check to guard validate (It would actually
be nice to have it enabled for tests whatever the log level, but the
performance hit is really big).
> Make sure assertions are enabled before calling VolcanoPlanner#validate
> ---
>
> Key: CALCITE-2537
> URL: https://issues.apache.org/jira/browse/CALCITE-2537
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> {{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
> But considering how enabling this method slows down the whole planning
> (changing it for running tests make the whole duration several hours), and
> this is some kind of assertion (since it throws {{AssertionError}}, I would
> suggest to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-2538) Missing RelMetadataQuery invalidation before subset cost improvement propagation
Laurent Goujon created CALCITE-2538:
---
Summary: Missing RelMetadataQuery invalidation before subset cost
improvement propagation
Key: CALCITE-2538
URL: https://issues.apache.org/jira/browse/CALCITE-2538
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Assignee: Julian Hyde
When planner log level is set to debug, several tests are failing with the
following error message:
{noformat}
rel
[rel#11408:EnumerableProject.ENUMERABLE.[](input=rel#11180:Subset#2.ENUMERABLE.[0],DUMMY=0)]
has lower cost {24.5 rows, 39.5 cpu, 0.0 io} than best cost {28.0 rows, 36.0
cpu, 0.0 io} of subset [rel#11177:Subset#3.ENUMERABLE.[]]
{noformat}
The following query (from 'sql/agg.iq') should exhibit the issue:
{code:sql}
select sum(e.sal) as s from "scott".emp e join "scott".emp m on e.mgr = e.empno;
{code}
I tried to trace it back and my understanding is that during cost improvement
propagation, a subset has its cost/best node changed, and it triggers
recomputing the cost of the parent subset, but the current best node of the
parent has its value cached in the current rel metadata query, and is not
recomputed. During a later validation, this error is detected and the assertion
is thrown.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-2537) Make sure assertions are enabled before calling VolcanoPlanner#validate
Laurent Goujon created CALCITE-2537:
---
Summary: Make sure assertions are enabled before calling
VolcanoPlanner#validate
Key: CALCITE-2537
URL: https://issues.apache.org/jira/browse/CALCITE-2537
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Laurent Goujon
Assignee: Julian Hyde
{{VolcanoPlanner#validate()}} is invoked if the log level is debug or lower.
But considering how enabling this method slows down the whole planning
(changing it for running tests make the whole duration several hours), and this
is some kind of assertion (since it throws {{AssertionError}}, I would suggest
to only enable it when assertions are enabled
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2471) RelNode description includes all tree when recomputed
[
https://issues.apache.org/jira/browse/CALCITE-2471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16582876#comment-16582876
]
Laurent Goujon commented on CALCITE-2471:
-
Added a simple test case, comparing description and digest strings.
> RelNode description includes all tree when recomputed
> -
>
> Key: CALCITE-2471
> URL: https://issues.apache.org/jira/browse/CALCITE-2471
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> The initial AbstractRelNode description is `{{typeName#id}}` but when
> recomputed, it includes all its inputs (and other attributes).
> When creating the final plan, HepPlanner visits each node bottom up, and
> recomputes the node digest. This causes node description to grow in size, up
> to the top node which contains basically a string copy of the whole tree. On
> large trees, this may consume a non-significant amount of memory.
> See email thread:
> https://lists.apache.org/thread.html/5e71c9f2b4e83865805841708f53471cfab1b5de673e6f162b68edc9@%3Cdev.calcite.apache.org%3E
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2471) RelNode description includes all tree when recomputed
[
https://issues.apache.org/jira/browse/CALCITE-2471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16582816#comment-16582816
]
Laurent Goujon commented on CALCITE-2471:
-
I wonder if doing a string match is a good enough test.
> RelNode description includes all tree when recomputed
> -
>
> Key: CALCITE-2471
> URL: https://issues.apache.org/jira/browse/CALCITE-2471
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> The initial AbstractRelNode description is `{{typeName#id}}` but when
> recomputed, it includes all its inputs (and other attributes).
> When creating the final plan, HepPlanner visits each node bottom up, and
> recomputes the node digest. This causes node description to grow in size, up
> to the top node which contains basically a string copy of the whole tree. On
> large trees, this may consume a non-significant amount of memory.
> See email thread:
> https://lists.apache.org/thread.html/5e71c9f2b4e83865805841708f53471cfab1b5de673e6f162b68edc9@%3Cdev.calcite.apache.org%3E
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2471) RelNode description includes all tree when recomputed
[
https://issues.apache.org/jira/browse/CALCITE-2471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16582791#comment-16582791
]
Laurent Goujon commented on CALCITE-2471:
-
Here's my pull request to address the issue:
https://github.com/apache/calcite/pull/795
> RelNode description includes all tree when recomputed
> -
>
> Key: CALCITE-2471
> URL: https://issues.apache.org/jira/browse/CALCITE-2471
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Major
>
> The initial AbstractRelNode description is `{{typeName#id}}` but when
> recomputed, it includes all its inputs (and other attributes).
> When creating the final plan, HepPlanner visits each node bottom up, and
> recomputes the node digest. This causes node description to grow in size, up
> to the top node which contains basically a string copy of the whole tree. On
> large trees, this may consume a non-significant amount of memory.
> See email thread:
> https://lists.apache.org/thread.html/5e71c9f2b4e83865805841708f53471cfab1b5de673e6f162b68edc9@%3Cdev.calcite.apache.org%3E
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-2471) RelNode description includes all tree when recomputed
Laurent Goujon created CALCITE-2471:
---
Summary: RelNode description includes all tree when recomputed
Key: CALCITE-2471
URL: https://issues.apache.org/jira/browse/CALCITE-2471
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Assignee: Julian Hyde
The initial AbstractRelNode description is `{{typeName#id}}` but when
recomputed, it includes all its inputs (and other attributes).
When creating the final plan, HepPlanner visits each node bottom up, and
recomputes the node digest. This causes node description to grow in size, up to
the top node which contains basically a string copy of the whole tree. On large
trees, this may consume a non-significant amount of memory.
See email thread:
https://lists.apache.org/thread.html/5e71c9f2b4e83865805841708f53471cfab1b5de673e6f162b68edc9@%3Cdev.calcite.apache.org%3E
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-2437) FilterMultiJoinMergeRule doesn't combine postFilterCondition
Laurent Goujon created CALCITE-2437:
---
Summary: FilterMultiJoinMergeRule doesn't combine
postFilterCondition
Key: CALCITE-2437
URL: https://issues.apache.org/jira/browse/CALCITE-2437
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Laurent Goujon
Assignee: Julian Hyde
{{FilterMultiJoinMergeRule}} merges a {{Filter}} on top of a {{MultiJoin}} into
the multi join itself as part of its post-join filter condition. But the rule
doesn't seem to combine the top filter with the existing condition.
I actually believe it's a bug in the code, although I never triggered it myself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
