[jira] [Commented] (CALCITE-6581) Incorrect INTERVAL math for WEEK and QUARTER
[
https://issues.apache.org/jira/browse/CALCITE-6581?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17881836#comment-17881836
]
Gian Merlino commented on CALCITE-6581:
---
Patch at: https://github.com/apache/calcite/pull/3968
> Incorrect INTERVAL math for WEEK and QUARTER
>
>
> Key: CALCITE-6581
> URL: https://issues.apache.org/jira/browse/CALCITE-6581
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
>
> WEEK and QUARTER intervals are incorrectly translated to millis and months.
> {{'X' WEEK}} is treated like {{'X' HOUR}}, and {{'X' QUARTER}} is treated
> like {{'X' MONTH}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (CALCITE-6581) Incorrect INTERVAL math for WEEK and QUARTER
Gian Merlino created CALCITE-6581:
-
Summary: Incorrect INTERVAL math for WEEK and QUARTER
Key: CALCITE-6581
URL: https://issues.apache.org/jira/browse/CALCITE-6581
Project: Calcite
Issue Type: Bug
Reporter: Gian Merlino
WEEK and QUARTER intervals are incorrectly translated to millis and months.
{{'X' WEEK}} is treated like {{'X' HOUR}}, and {{'X' QUARTER}} is treated like
{{'X' MONTH}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5763) Discontinue support for Guava < 20.0
[ https://issues.apache.org/jira/browse/CALCITE-5763?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17775821#comment-17775821 ] Gian Merlino commented on CALCITE-5763: --- [~julianhyde] yes, please go ahead. Druid is on Calcite 1.35 now, and recently decided to drop support for Hadoop 2 and update to Guava 31.1-jre. That means we will be able to update to future Calcite releases that do not support older Guavas. > Discontinue support for Guava < 20.0 > > > Key: CALCITE-5763 > URL: https://issues.apache.org/jira/browse/CALCITE-5763 > Project: Calcite > Issue Type: Bug >Reporter: Julian Hyde >Assignee: Julian Hyde >Priority: Major > Labels: pull-request-available > Fix For: 1.36.0 > > > Discontinue support for Guava versions before 20.0, and resume building on > the latest Guava. This reverses CALCITE-5477, which changes the build from > Guava 31.1-jre to 19.0, and CALCITE-5428, which moves the minimum supported > Guava version from 19.0 to 16.0.1. > This change will happen no earlier than "the first release after August", > therefore can be merged to main no earlier than 2023-09-01. I recommend that > it is merged very soon after that date. I have set fixVersion = 1.36 assuming > that 1.36 is the first release after August. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
[
https://issues.apache.org/jira/browse/CALCITE-5865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17745688#comment-17745688
]
Gian Merlino commented on CALCITE-5865:
---
Sorry about that! Strangely, the original PR didn't have any failures show up,
although it does look like some checks were skipped:
https://github.com/apache/calcite/pull/3327/checks. I am not sure why that
happened. Thanks for fixing it.
> ClassCastException with FLOOR and CEIL on conformances that are not builtin
> ---
>
> Key: CALCITE-5865
> URL: https://issues.apache.org/jira/browse/CALCITE-5865
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Blocker
> Labels: pull-request-available
> Fix For: 1.35.0
>
>
> CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that
> is keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However,
> it was implemented in such a way that implementations of {{SqlConformance}}
> that are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due
> to this line in {{Parser.jj}}:
> {code}
> SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag,
> (SqlConformanceEnum) this.conformance);
> {code}
> Better would be to pass {{this.conformance}} without casting, and adjust
> {{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
> This affects us in Apache Druid land, because we have a conformance that
> extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Resolved] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
[
https://issues.apache.org/jira/browse/CALCITE-5865?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-5865.
---
Resolution: Fixed
> ClassCastException with FLOOR and CEIL on conformances that are not builtin
> ---
>
> Key: CALCITE-5865
> URL: https://issues.apache.org/jira/browse/CALCITE-5865
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Blocker
> Labels: pull-request-available
> Fix For: 1.35.0
>
>
> CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that
> is keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However,
> it was implemented in such a way that implementations of {{SqlConformance}}
> that are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due
> to this line in {{Parser.jj}}:
> {code}
> SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag,
> (SqlConformanceEnum) this.conformance);
> {code}
> Better would be to pass {{this.conformance}} without casting, and adjust
> {{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
> This affects us in Apache Druid land, because we have a conformance that
> extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
[
https://issues.apache.org/jira/browse/CALCITE-5865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17745179#comment-17745179
]
Gian Merlino commented on CALCITE-5865:
---
I committed the small fix to {{main}}, so I'll mark this bug resolved. Based on
the discussion on the rc2 vote thread, I marked this for 1.35.0 since it's
expected to be included in rc3. [~xiong] please let me know if you need
anything from me on that.
> ClassCastException with FLOOR and CEIL on conformances that are not builtin
> ---
>
> Key: CALCITE-5865
> URL: https://issues.apache.org/jira/browse/CALCITE-5865
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Blocker
> Labels: pull-request-available
> Fix For: 1.35.0
>
>
> CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that
> is keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However,
> it was implemented in such a way that implementations of {{SqlConformance}}
> that are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due
> to this line in {{Parser.jj}}:
> {code}
> SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag,
> (SqlConformanceEnum) this.conformance);
> {code}
> Better would be to pass {{this.conformance}} without casting, and adjust
> {{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
> This affects us in Apache Druid land, because we have a conformance that
> extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
[
https://issues.apache.org/jira/browse/CALCITE-5865?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5865:
--
Fix Version/s: 1.35.0
> ClassCastException with FLOOR and CEIL on conformances that are not builtin
> ---
>
> Key: CALCITE-5865
> URL: https://issues.apache.org/jira/browse/CALCITE-5865
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Blocker
> Labels: pull-request-available
> Fix For: 1.35.0
>
>
> CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that
> is keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However,
> it was implemented in such a way that implementations of {{SqlConformance}}
> that are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due
> to this line in {{Parser.jj}}:
> {code}
> SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag,
> (SqlConformanceEnum) this.conformance);
> {code}
> Better would be to pass {{this.conformance}} without casting, and adjust
> {{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
> This affects us in Apache Druid land, because we have a conformance that
> extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
[
https://issues.apache.org/jira/browse/CALCITE-5865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17744846#comment-17744846
]
Gian Merlino commented on CALCITE-5865:
---
Thank you!
> ClassCastException with FLOOR and CEIL on conformances that are not builtin
> ---
>
> Key: CALCITE-5865
> URL: https://issues.apache.org/jira/browse/CALCITE-5865
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
>
> CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that
> is keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However,
> it was implemented in such a way that implementations of {{SqlConformance}}
> that are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due
> to this line in {{Parser.jj}}:
> {code}
> SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag,
> (SqlConformanceEnum) this.conformance);
> {code}
> Better would be to pass {{this.conformance}} without casting, and adjust
> {{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
> This affects us in Apache Druid land, because we have a conformance that
> extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
[
https://issues.apache.org/jira/browse/CALCITE-5865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17744817#comment-17744817
]
Gian Merlino commented on CALCITE-5865:
---
PR in https://github.com/apache/calcite/pull/3327.
Btw, to me this seems more like a {{RelDataTypeSystem}} thing than a
{{SqlConformance}} thing. It seems similar to {{deriveSumType}},
{{deriveAvgAggType}}, etc. However, in the PR, I stuck with the conformance
approach, for two reasons:
- I'm asking for this to be included in 1.35.0, which is up to rc2 already, so
I'm keeping the patch small.
- I could be missing this reason why this should be a conformance thing.
> ClassCastException with FLOOR and CEIL on conformances that are not builtin
> ---
>
> Key: CALCITE-5865
> URL: https://issues.apache.org/jira/browse/CALCITE-5865
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
>
> CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that
> is keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However,
> it was implemented in such a way that implementations of {{SqlConformance}}
> that are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due
> to this line in {{Parser.jj}}:
> {code}
> SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag,
> (SqlConformanceEnum) this.conformance);
> {code}
> Better would be to pass {{this.conformance}} without casting, and adjust
> {{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
> This affects us in Apache Druid land, because we have a conformance that
> extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (CALCITE-5865) ClassCastException with FLOOR and CEIL on conformances that are not builtin
Gian Merlino created CALCITE-5865:
-
Summary: ClassCastException with FLOOR and CEIL on conformances
that are not builtin
Key: CALCITE-5865
URL: https://issues.apache.org/jira/browse/CALCITE-5865
Project: Calcite
Issue Type: Bug
Reporter: Gian Merlino
CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that is
keyed off the conformance being {{SqlConformanceEnum.BIG_QUERY}}. However, it
was implemented in such a way that implementations of {{SqlConformance}} that
are not {{SqlConformanceEnum}} would throw {{ClassCastException}}, due to this
line in {{Parser.jj}}:
{code}
SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag, (SqlConformanceEnum)
this.conformance);
{code}
Better would be to pass {{this.conformance}} without casting, and adjust
{{SqlStdOperatorTable.floorCeil}} to handle any kind of conformance.
This affects us in Apache Druid land, because we have a conformance that
extends {{SqlAbstractConformance}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5479) FamilyOperandTypeChecker is not readily composable in sequences
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17741833#comment-17741833
]
Gian Merlino commented on CALCITE-5479:
---
Thank you!
> FamilyOperandTypeChecker is not readily composable in sequences
> ---
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5479) FamilyOperandTypeChecker is not readily composable in sequences
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17741810#comment-17741810
]
Gian Merlino commented on CALCITE-5479:
---
[~julianhyde], do you think you'll have a chance to look at this before 1.35 is
cut?
> FamilyOperandTypeChecker is not readily composable in sequences
> ---
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5708) SUBSTRING validation error if any parameter is a NULL literal
[
https://issues.apache.org/jira/browse/CALCITE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17740235#comment-17740235
]
Gian Merlino commented on CALCITE-5708:
---
Committed in
https://github.com/apache/calcite/commit/39e5c764e76f46ec2ff960c07bb72dc370c747fc.
> SUBSTRING validation error if any parameter is a NULL literal
> -
>
> Key: CALCITE-5708
> URL: https://issues.apache.org/jira/browse/CALCITE-5708
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.34.0
>Reporter: Evgeny Stanilovsky
>Assignee: Evgeny Stanilovsky
>Priority: Major
> Labels: patch-available, pull-request-available
> Fix For: 1.35.0
>
>
> According to standard:
> {noformat}
> 6.18
> ...
> 3)If is specified, then:
> ...
> c) If either C, S, or L is the null value, then the result of the substring function> is
> the null value.
> {noformat}
> calcite not follow this rule for now.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Resolved] (CALCITE-5708) SUBSTRING validation error if any parameter is a NULL literal
[
https://issues.apache.org/jira/browse/CALCITE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-5708.
---
Resolution: Fixed
> SUBSTRING validation error if any parameter is a NULL literal
> -
>
> Key: CALCITE-5708
> URL: https://issues.apache.org/jira/browse/CALCITE-5708
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.34.0
>Reporter: Evgeny Stanilovsky
>Assignee: Evgeny Stanilovsky
>Priority: Major
> Labels: patch-available, pull-request-available
> Fix For: 1.35.0
>
>
> According to standard:
> {noformat}
> 6.18
> ...
> 3)If is specified, then:
> ...
> c) If either C, S, or L is the null value, then the result of the substring function> is
> the null value.
> {noformat}
> calcite not follow this rule for now.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5708) SUBSTRING validation error if any parameter is a NULL literal
[
https://issues.apache.org/jira/browse/CALCITE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5708:
--
Summary: SUBSTRING validation error if any parameter is a NULL literal
(was: Change SUBSTRING result if either of parameters is NULL literal)
> SUBSTRING validation error if any parameter is a NULL literal
> -
>
> Key: CALCITE-5708
> URL: https://issues.apache.org/jira/browse/CALCITE-5708
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.34.0
>Reporter: Evgeny Stanilovsky
>Assignee: Evgeny Stanilovsky
>Priority: Major
> Labels: patch-available, pull-request-available
> Fix For: 1.35.0
>
>
> According to standard:
> {noformat}
> 6.18
> ...
> 3)If is specified, then:
> ...
> c) If either C, S, or L is the null value, then the result of the substring function> is
> the null value.
> {noformat}
> calcite not follow this rule for now.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Resolved] (CALCITE-5727) RelOptFixture#checkUnchanged should assert planAfter is not present
[
https://issues.apache.org/jira/browse/CALCITE-5727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-5727.
---
Resolution: Fixed
> RelOptFixture#checkUnchanged should assert planAfter is not present
> ---
>
> Key: CALCITE-5727
> URL: https://issues.apache.org/jira/browse/CALCITE-5727
> Project: Calcite
> Issue Type: Improvement
> Components: tests
>Affects Versions: 1.34.0
>Reporter: Benchao Li
>Assignee: Benchao Li
>Priority: Minor
> Labels: pull-request-available
> Fix For: 1.35.0
>
>
> This is discovered while I'm review another
> [PR|https://github.com/apache/calcite/pull/3193#discussion_r1204998877]. And
> there are already some tests that using {{{}checkUnchanged{}}}, but the xml
> file still contains {{planAfter}} section, such as
> {{{}RelOptRulesTest#testNoReduceAverage{}}}.
> IMO, there is no need to keep {{planAfter}} for {{{}checkUnchanged{}}}, and
> it does more harm than good to keep it. Even we can make {{planAfter}} the
> same with {{{}planBefore{}}}, it will bring burden for reading (because we'll
> identify the difference of planAfter and planBefore). And it will be more
> harmful when {{planAfter}} is different from {{{}planBefore{}}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5708) Change SUBSTRING result if either of parameters is NULL literal
[
https://issues.apache.org/jira/browse/CALCITE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17739231#comment-17739231
]
Gian Merlino commented on CALCITE-5708:
---
I wrote a review as well. Hope this helps.
There is a question about whether a similar problem exists for other functions.
I haven't tested it myself, but from reading the code, it seems like when
{{typeCoercion}} is {{true}} (which is the default), the
{{FamilyOperandTypeChecker}} allows {{NULL}} literals. So, hopefully, this
problem is limited to {{SUBSTRING}} because it has some custom checking logic
that specifically didn't allow for {{NULL}} literals.
> Change SUBSTRING result if either of parameters is NULL literal
> ---
>
> Key: CALCITE-5708
> URL: https://issues.apache.org/jira/browse/CALCITE-5708
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.34.0
>Reporter: Evgeny Stanilovsky
>Assignee: Evgeny Stanilovsky
>Priority: Major
> Labels: patch-available, pull-request-available
> Fix For: 1.35.0
>
>
> According to standard:
> {noformat}
> 6.18
> ...
> 3)If is specified, then:
> ...
> c) If either C, S, or L is the null value, then the result of the substring function> is
> the null value.
> {noformat}
> calcite not follow this rule for now.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5479) FamilyOperandTypeChecker is not readily composable in sequences
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17736296#comment-17736296
]
Gian Merlino commented on CALCITE-5479:
---
I believe so. The main changes are these two:
-
https://github.com/apache/calcite/pull/3030/files#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L315-R315
-
https://github.com/apache/calcite/pull/3030/files#diff-2d686a3a894dffd63d982ee79961f6abe290f505d8cac4516fd3abf184cb565eL67-R67
Which have the effect of setting {{iFormalOperand}} properly in strictly more
cases.
The rest of the patch is about modifying {{FamilyOperandTypeChecker}} such that
it ignores {{iFormalOperand}} when run as a single-operand checker. That is
what allows us to remove the code that specifically sets it to zero for
{{FamilyOperandTypeChecker.class}}. It is also what fixes the original bug that
inspired me to raise this case: {{FamilyOperandTypeChecker}} would not work
properly when composed inside another checker.
> FamilyOperandTypeChecker is not readily composable in sequences
> ---
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Resolved] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-5477.
---
Resolution: Fixed
Committed the patch. Per CALCITE-5763 this is expected to be reverted by
2023-09-01.
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17730283#comment-17730283
]
Gian Merlino commented on CALCITE-5477:
---
OK, I'll proceed with the plan to build with Guava 19, and am okay with the
compromise to revert that change in the next release.
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17730224#comment-17730224
]
Gian Merlino commented on CALCITE-5477:
---
Speaking as a Druid committer, my preference is that we can use release builds
of Calcite that are compatible with Guava 16.0.1, which we are stuck on for now
due to a relatively small, but influential and vocal sector of the community
that is interested in support for Hadoop 2. And as anyone that worked with
Hadoop 2 knows— it has a way of keeping you stuck on old versions of things. I
think we will be able to resolve this situation with some time and effort, but
I would prefer it not block our ability to upgrade Calcite too!
Speaking as a Calcite committer, I wonder if the nicest solution is
internalizing the problematic Guava code: meaning copy `Preconditions` into the
Calcite codebase itself. That way we get the performance benefits of the new
overloads, can still build with guava 31, and can offer compatibility with
ancient guavas to people who are interested in that kind of thing.
Thoughts?
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5477:
--
Fix Version/s: 1.35.0
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5479) FamilyOperandTypeChecker is not readily composable in sequences
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5479:
--
Fix Version/s: 1.35.0
> FamilyOperandTypeChecker is not readily composable in sequences
> ---
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.35.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17729990#comment-17729990
]
Gian Merlino commented on CALCITE-5477:
---
New PR: https://github.com/apache/calcite/pull/3249
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-3877) In RexWindow, make fields upperBound and lowerBound not-nullable
[
https://issues.apache.org/jira/browse/CALCITE-3877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17695424#comment-17695424
]
Gian Merlino commented on CALCITE-3877:
---
I wanted to inquire about this block added in SqlToRelConverter as part of this
patch:
{code}
if (!aggCall.getOperator().allowsFraming()) {
// If the operator does not allow framing, bracketing is implicitly
// everything up to the current row.
sqlLowerBound = SqlWindow.createUnboundedPreceding(SqlParserPos.ZERO);
sqlUpperBound = SqlWindow.createCurrentRow(SqlParserPos.ZERO);
if (aggCall.getKind() == SqlKind.ROW_NUMBER) {
// ROW_NUMBER() expects specific kind of framing.
rows = true;
}
}
{code}
The comment and logic doesn't seem right. The operators that do not allow
framing are {{LAG}}, {{LEAD}}, {{RANK}}, {{ROW_NUMBER}}, {{PERCENT_RANK}},
{{DENSE_RANK}}, and {{CUME_DIST}}. However certain of these operators, like
{{LAG}} and {{LEAD}}, are meant to "see" the entire window, including rows
beyond the current row.
Would it make sense for me to raise a patch changing the upper bound in all
these cases to unbounded following?
Btw, I noticed this when upgrading to the latest version of Calcite, and
noticing that the default frames changed for {{LAG}} and {{LEAD}}. It was
formerly unbounded preceding to unbounded following.
> In RexWindow, make fields upperBound and lowerBound not-nullable
>
>
> Key: CALCITE-3877
> URL: https://issues.apache.org/jira/browse/CALCITE-3877
> Project: Calcite
> Issue Type: Bug
>Reporter: Steven Talbot
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.23.0
>
>
> Null lowerBound/upperBond in RexWindow causes NPE in RexShuttle. Here's a
> test that can be dropped into RexShuttleTest.java:
> {code:java}
> @Test public void handlesNullBoundsInRexWindow() {
> final RelBuilder builder =
> RelBuilder.create(RelBuilderTest.config().build());
> final RexNode over = builder.getRexBuilder().makeOver(
> builder.getTypeFactory().createSqlType(SqlTypeName.BIGINT),
> SqlStdOperatorTable.ROW_NUMBER,
> ImmutableList.of(),
> ImmutableList.of(),
> ImmutableList.of(),
> null,
> null,
> true,
> true,
> true,
> true,
>true
> );
> assertThat(over.accept(new RexShuttle()), is(over));
> }
> {code}
> That will raise an NPE. Fix is to handle NULLs when doing the child accept on
> the RexWindowBound[s] in RexShuttle.visitWindow.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17691732#comment-17691732
]
Gian Merlino commented on CALCITE-5477:
---
That sounds fair enough. I will try this approach out, and then update my PR to
do as you describe. Afterwards, I'll also do the best I can to get us past the
Guava 16 stuff in Druid world.
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5477) Compatibility with Guava < 20.0
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5477:
--
Summary: Compatibility with Guava < 20.0 (was: Prefer Util.checkArgument
over Preconditions.checkArgument)
> Compatibility with Guava < 20.0
> ---
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Prefer Util.checkArgument over Preconditions.checkArgument
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17690664#comment-17690664
]
Gian Merlino commented on CALCITE-5477:
---
Sorry for the delay in responding. I was out of office for a bit, but I am back
at work now.
One other possible solution is to copy the latest version of Preconditions from
Guava into Calcite itself, and use that instead of the one from Guava. Then,
we'd get the benefit of the non-boxing overloads, as well as the benefit of
compatibility with a wider range of Guava versions. What do you think of that
option?
> Prefer Util.checkArgument over Preconditions.checkArgument
> --
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5479) FamilyOperandTypeChecker is not readily composable in sequences
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17690605#comment-17690605
]
Gian Merlino commented on CALCITE-5479:
---
I see. I've updated the linked PR with what I think is a better fix, based on
this discussion.
Remarks on the new fix: I believe the problem is that FamilyOperandTypeChecker
is used in two different styles.
First:
{code:java}
OperandTypes.sequence(OperandTypes.STRING, OperandTypes.NUMERIC)
{code}
In this style, two FamilyOperandTypeCheckers, STRING and NUMERIC, are wrapped
in a CompositeOperandTypeChecker. For this to work with the current
implementation of FamilyOperandTypeChecker, {{iFormalOperand}} must be zero. I
suppose this is why CompositeOperandTypeChecker has some logic that sets it to
zero specifically for FamilyOperandTypeChecker.
Second:
{code:java}
OperandTypes.family(SqlTypeFamily.STRING, SqlTypeFamily.NUMERIC)
{code}
In this style, there is a single FamilyOperandTypeChecker which behaves the
same as the composite sequence above. For this to work, the
FamilyOperandTypeCheckers do need to be aware of the operand positions.
These two styles are difficult to reconcile, hence the cracks that need
papering.
The fix in the updated PR is to:
# Have {{CompositeOperandTypeChecker}} send down {{ord.i}} for
{{iFormalOperand}} always.
# Have {{FamilyOperandTypeChecker#checkSingleOperandType}}, the method from
the {{SqlSingleOperandTypeChecker}} interface, ignore {{iFormalOperand}}. So,
when the checker is being used as a {{SqlSingleOperandTypeChecker}} — as it is
inside a sequence — it ignores operand position and always uses the zeroth
family. I cannot think of a valid reason for it to do anything else, given how
it is typically composed into sequences.
# {{FamilyOperandTypeChecker}} continues being aware of operand position when
used as a {{SqlOperandTypeChecker}} via {{checkOperandTypes}}.
> FamilyOperandTypeChecker is not readily composable in sequences
> ---
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5479) FamilyOperandTypeChecker is not readily composable in sequences
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5479:
--
Summary: FamilyOperandTypeChecker is not readily composable in sequences
(was: Restore consistent handling of iFormalOperand in sequence checkers)
> FamilyOperandTypeChecker is not readily composable in sequences
> ---
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5478) Use highest input precision for datetimes in SqlTypeFactoryImpl.leastRestrictive
[
https://issues.apache.org/jira/browse/CALCITE-5478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17690553#comment-17690553
]
Gian Merlino commented on CALCITE-5478:
---
Thanks for your comment. Sorry for the delay in responding. I was out of office
for a bit, but I am back at work now.
The PR I linked implements simpler logic for datetimes, rather than treating
datetimes and decimals the same way. I added a couple of tests for it. I
believe the logic is correct: if both types under consideration are datetimes,
and have same type name, then take the max precision across the two types.
> Use highest input precision for datetimes in
> SqlTypeFactoryImpl.leastRestrictive
>
>
> Key: CALCITE-5478
> URL: https://issues.apache.org/jira/browse/CALCITE-5478
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> For example: {{leastRestrictive(TIMESTAMP(3), TIMESTAMP(0))}} would be
> {{TIMESTAMP(3)}}, not {{TIMESTAMP(0)}}.
> Some code exists to do this for other types, but not datetimes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Comment Edited] (CALCITE-5477) Prefer Util.checkArgument over Preconditions.checkArgument
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17677581#comment-17677581
]
Gian Merlino edited comment on CALCITE-5477 at 1/17/23 8:34 PM:
The situation is a little awkward due to the new overloads. I believe the
possible solutions are:
- The one in the linked PR, involving an additional level of indirection.
- Make release builds of Calcite using an older version of Guava (prior to
20.0). People could always drop in a newer version instead, but this may be
annoying for downstream consumers.
- Avoiding the new overloads, using {{forbidden-apis}} plus some code changes.
With the third approach, code changes are needed because the new overloads
capture calls that previously went to a varags {{Object...}} method. An example
of the problem is this call in DateString:
{code:java}
Preconditions.checkArgument(PATTERN.matcher(v).matches(),
"Invalid date format:", v);
{code}
{{v}} is a {{{}Object{}}}. If built against Guava prior to 20.0, this goes to
{{{}checkArgument(boolean, String, Object...){}}}. However, when building
against later versions of Guava, this goes to {{{}checkArgument(boolean,
String, Object){}}}. This method doesn't exist in Guava prior to 20.0, which
leads to a method-not-found error at runtime.
So, to solve that, all such calls would need to be replaced with explicit
{{Object[]}} creation, such as:
{code:java}
Preconditions.checkArgument(PATTERN.matcher(v).matches(),
"Invalid date format:", new Object[]{v});
{code}
Definitely open to any solution that seems best.
was (Author: gian):
The situation is a little awkward due to the new overloads. I believe the
possible solutions are:
- The one in the linked PR, involving an additional level of indirection.
- Build release builds of Calcite using an older version of Guava (prior to
20.0). People could always drop in a newer version instead, but this may be
annoying for downstream consumers.
- Avoiding the new overloads, using {{forbidden-apis}} plus some code changes.
With the third approach, code changes are needed because the new overloads
capture calls that previously went to a varags {{Object...}} method. An example
of the problem is this call in DateString:
{code:java}
Preconditions.checkArgument(PATTERN.matcher(v).matches(),
"Invalid date format:", v);
{code}
{{v}} is a {{{}Object{}}}. If built against Guava prior to 20.0, this goes to
{{{}checkArgument(boolean, String, Object...){}}}. However, when building
against later versions of Guava, this goes to {{{}checkArgument(boolean,
String, Object){}}}. This method doesn't exist in Guava prior to 20.0, which
leads to a method-not-found error at runtime.
So, to solve that, all such calls would need to be replaced with explicit
{{Object[]}} creation, such as:
{code:java}
Preconditions.checkArgument(PATTERN.matcher(v).matches(),
"Invalid date format:", new Object[]{v});
{code}
Definitely open to any solution that seems best.
> Prefer Util.checkArgument over Preconditions.checkArgument
> --
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Prefer Util.checkArgument over Preconditions.checkArgument
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17677581#comment-17677581
]
Gian Merlino commented on CALCITE-5477:
---
The situation is a little awkward due to the new overloads. I believe the
possible solutions are:
- The one in the linked PR, involving an additional level of indirection.
- Build release builds of Calcite using an older version of Guava (prior to
20.0). People could always drop in a newer version instead, but this may be
annoying for downstream consumers.
- Avoiding the new overloads, using {{forbidden-apis}} plus some code changes.
With the third approach, code changes are needed because the new overloads
capture calls that previously went to a varags {{Object...}} method. An example
of the problem is this call in DateString:
{code:java}
Preconditions.checkArgument(PATTERN.matcher(v).matches(),
"Invalid date format:", v);
{code}
{{v}} is a {{{}Object{}}}. If built against Guava prior to 20.0, this goes to
{{{}checkArgument(boolean, String, Object...){}}}. However, when building
against later versions of Guava, this goes to {{{}checkArgument(boolean,
String, Object){}}}. This method doesn't exist in Guava prior to 20.0, which
leads to a method-not-found error at runtime.
So, to solve that, all such calls would need to be replaced with explicit
{{Object[]}} creation, such as:
{code:java}
Preconditions.checkArgument(PATTERN.matcher(v).matches(),
"Invalid date format:", new Object[]{v});
{code}
Definitely open to any solution that seems best.
> Prefer Util.checkArgument over Preconditions.checkArgument
> --
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Updated] (CALCITE-5477) Prefer Util.checkArgument over Preconditions.checkArgument
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-5477:
--
Description:
Related to CALCITE-5428.
Preconditions.checkArgument gained new overloads in Guava 20.0, which prevents
code built using Guava 20.0 from running properly with earlier versions of
Guava. (When building against a later version of Guava, as Calcite does by
default, the Java compiler generates calls to methods that don't exist in those
earlier versions.)
Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
earlier versions this would be handled by {{checkArgument(boolean, String,
Object...)}}.
was:
Related to CALCITE-5428.
Preconditions.checkArgument gained new overloads in Guava 20.0, which prevents
code built using Guava 20.0 from running properly with earlier versions of
Guava. (When building against a later version of Guava, as Calcite does by
default, the Java compiler generates calls to methods that don't exist in those
earlier versions.)
Example: in Guava 20.0 there is {{checkArgument(String, Object, int)}}; in
earlier versions this would be handled by {{checkArgument(String, Object,
Object...)}}.
> Prefer Util.checkArgument over Preconditions.checkArgument
> --
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(boolean, String, int)}}; in
> earlier versions this would be handled by {{checkArgument(boolean, String,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5479) Restore consistent handling of iFormalOperand in sequence checkers
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17676980#comment-17676980
]
Gian Merlino commented on CALCITE-5479:
---
Ah. Would you mind explaining what the behavior is supposed to be? Maybe I
don't understand how {{OperandTypes.sequence}} is supposed to work. The stuff
we're doing in Druid that stopped working is like:
{code}
OperandTypes.sequence(
"F(expr, path)",
OperandTypes.ANY,
OperandTypes.and(OperandTypes.family(SqlTypeFamily.STRING),
OperandTypes.LITERAL)
)
{code}
That's for an operator that accepts an {{expr}}, which can be any type, and a
{{path}}, which must be a string literal.
The sequence checker passes {{iFormalOperand = 1}} to the {{and}} checker. Then
the checker {{OperandTypes.family(SqlTypeFamily.STRING)}} throws an error on
{{families.get(iFormalOperand)}}, because {{iFormalOperand}} is {{1}} but
{{families}} doesn't have that many elements.
If we're using the APIs wrong, please let me know since I'd rather fix that on
our end.
> Restore consistent handling of iFormalOperand in sequence checkers
> --
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5479) Restore consistent handling of iFormalOperand in sequence checkers
[
https://issues.apache.org/jira/browse/CALCITE-5479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17676871#comment-17676871
]
Gian Merlino commented on CALCITE-5479:
---
PR: https://github.com/apache/calcite/pull/3030
> Restore consistent handling of iFormalOperand in sequence checkers
> --
>
> Key: CALCITE-5479
> URL: https://issues.apache.org/jira/browse/CALCITE-5479
> Project: Calcite
> Issue Type: Bug
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Handling for {{OperandTypes.sequence}} changed in
> [33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
> such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
> but is instead:
> - Zero if the subchecker is {{FamilyOperandTypeChecker}}.
> - Otherwise, the operand number in the overall sequence.
> It causes problems for the way we're using sequence checkers in Druid, since
> we don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
> behavior: that {{iFormalOperand}} is always zero, and therefore we can put
> any checker into the sequence without it being "aware" that it is in a
> sequence.
> I marked this as a bug in case this change was made accidentally. If it was
> made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (CALCITE-5479) Restore consistent handling of iFormalOperand in sequence checkers
Gian Merlino created CALCITE-5479:
-
Summary: Restore consistent handling of iFormalOperand in sequence
checkers
Key: CALCITE-5479
URL: https://issues.apache.org/jira/browse/CALCITE-5479
Project: Calcite
Issue Type: Bug
Reporter: Gian Merlino
Handling for {{OperandTypes.sequence}} changed in
[33f4ab40bbee26e06209061c35a422f2f1e05371|https://github.com/apache/calcite/commit/33f4ab40bbee26e06209061c35a422f2f1e05371#diff-b0b8d58a792b8e60b9e97717912aecfc6695536f5026ac4d5231d14e34b91566L303-R316]
such that {{iFormalOperand}} passed to subcheckers is no longer always zero,
but is instead:
- Zero if the subchecker is {{FamilyOperandTypeChecker}}.
- Otherwise, the operand number in the overall sequence.
It causes problems for the way we're using sequence checkers in Druid, since we
don't always use {{FamilyOperandTypeChecker}}, but we _do_ assume the old
behavior: that {{iFormalOperand}} is always zero, and therefore we can put any
checker into the sequence without it being "aware" that it is in a sequence.
I marked this as a bug in case this change was made accidentally. If it was
made for a reason, please let me know. Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5478) Use highest input precision for datetimes in SqlTypeFactoryImpl.leastRestrictive
[
https://issues.apache.org/jira/browse/CALCITE-5478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17676868#comment-17676868
]
Gian Merlino commented on CALCITE-5478:
---
https://github.com/apache/calcite/pull/3029 makes this modification.
> Use highest input precision for datetimes in
> SqlTypeFactoryImpl.leastRestrictive
>
>
> Key: CALCITE-5478
> URL: https://issues.apache.org/jira/browse/CALCITE-5478
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> For example: {{leastRestrictive(TIMESTAMP(3), TIMESTAMP(0))}} would be
> {{TIMESTAMP(3)}}, not {{TIMESTAMP(0)}}.
> Some code exists to do this for other types, but not datetimes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (CALCITE-5478) Use highest input precision for datetimes in SqlTypeFactoryImpl.leastRestrictive
Gian Merlino created CALCITE-5478:
-
Summary: Use highest input precision for datetimes in
SqlTypeFactoryImpl.leastRestrictive
Key: CALCITE-5478
URL: https://issues.apache.org/jira/browse/CALCITE-5478
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Gian Merlino
For example: {{leastRestrictive(TIMESTAMP(3), TIMESTAMP(0))}} would be
{{TIMESTAMP(3)}}, not {{TIMESTAMP(0)}}.
Some code exists to do this for other types, but not datetimes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (CALCITE-5477) Prefer Util.checkArgument over Preconditions.checkArgument
[
https://issues.apache.org/jira/browse/CALCITE-5477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17676866#comment-17676866
]
Gian Merlino commented on CALCITE-5477:
---
https://github.com/apache/calcite/pull/3028 adds Util.checkArgument and
migrates existing usages.
> Prefer Util.checkArgument over Preconditions.checkArgument
> --
>
> Key: CALCITE-5477
> URL: https://issues.apache.org/jira/browse/CALCITE-5477
> Project: Calcite
> Issue Type: Improvement
>Reporter: Gian Merlino
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Related to CALCITE-5428.
> Preconditions.checkArgument gained new overloads in Guava 20.0, which
> prevents code built using Guava 20.0 from running properly with earlier
> versions of Guava. (When building against a later version of Guava, as
> Calcite does by default, the Java compiler generates calls to methods that
> don't exist in those earlier versions.)
> Example: in Guava 20.0 there is {{checkArgument(String, Object, int)}}; in
> earlier versions this would be handled by {{checkArgument(String, Object,
> Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (CALCITE-5477) Prefer Util.checkArgument over Preconditions.checkArgument
Gian Merlino created CALCITE-5477:
-
Summary: Prefer Util.checkArgument over Preconditions.checkArgument
Key: CALCITE-5477
URL: https://issues.apache.org/jira/browse/CALCITE-5477
Project: Calcite
Issue Type: Improvement
Reporter: Gian Merlino
Related to CALCITE-5428.
Preconditions.checkArgument gained new overloads in Guava 20.0, which prevents
code built using Guava 20.0 from running properly with earlier versions of
Guava. (When building against a later version of Guava, as Calcite does by
default, the Java compiler generates calls to methods that don't exist in those
earlier versions.)
Example: in Guava 20.0 there is {{checkArgument(String, Object, int)}}; in
earlier versions this would be handled by {{checkArgument(String, Object,
Object...)}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (CALCITE-5428) Reduce minimum Guava version to 16.0.1.
Gian Merlino created CALCITE-5428: - Summary: Reduce minimum Guava version to 16.0.1. Key: CALCITE-5428 URL: https://issues.apache.org/jira/browse/CALCITE-5428 Project: Calcite Issue Type: Task Components: core Reporter: Gian Merlino If Calcite were to support Guava 16.0.1, this would help Druid upgrade to the latest version while maintaining compatibility with Hadoop 2.x. See discussion in: https://lists.apache.org/thread/8g5fpd4kpt7zhvf4l9397p9k95fny8tq. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (CALCITE-5428) Reduce minimum Guava version to 16.0.1
[ https://issues.apache.org/jira/browse/CALCITE-5428?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gian Merlino updated CALCITE-5428: -- Summary: Reduce minimum Guava version to 16.0.1 (was: Reduce minimum Guava version to 16.0.1.) > Reduce minimum Guava version to 16.0.1 > -- > > Key: CALCITE-5428 > URL: https://issues.apache.org/jira/browse/CALCITE-5428 > Project: Calcite > Issue Type: Task > Components: core >Reporter: Gian Merlino >Priority: Major > > If Calcite were to support Guava 16.0.1, this would help Druid upgrade to the > latest version while maintaining compatibility with Hadoop 2.x. > See discussion in: > https://lists.apache.org/thread/8g5fpd4kpt7zhvf4l9397p9k95fny8tq. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (CALCITE-4121) Avatica misplaces properties from URL while connecting
Gian Merlino created CALCITE-4121:
-
Summary: Avatica misplaces properties from URL while connecting
Key: CALCITE-4121
URL: https://issues.apache.org/jira/browse/CALCITE-4121
Project: Calcite
Issue Type: Bug
Components: avatica
Reporter: Gian Merlino
Fix For: avatica-1.17.0
Avatica's driver goes through some effort to extract properties from the JDBC
URL, but then loses them because it doesn't pass them to the
{{OpenConnectionRequest}}:
https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/java/org/apache/calcite/avatica/remote/Driver.java#L163-L181.
I think the fix should be passing {{conn.info}} instead of {{info}}, because
URL properties are added to {{conn.info}} in {{super.connect(url, info)}}.
The fix looks simple enough — any suggestions on the best way to add tests for
it?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3178) RexSimplify.simplifyOrTerms slow with large OR filters
[
https://issues.apache.org/jira/browse/CALCITE-3178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16879907#comment-16879907
]
Gian Merlino commented on CALCITE-3178:
---
What's the best way to add a test case for a performance thing like that?
Should I add a case that checks for the correct result with a relatively short
timeout (a few seconds), so it's expected to fail in master, but potentially
succeed if the algorithm is improved? Any suggestion for the test class that
the test case should go into?
> RexSimplify.simplifyOrTerms slow with large OR filters
> --
>
> Key: CALCITE-3178
> URL: https://issues.apache.org/jira/browse/CALCITE-3178
> Project: Calcite
> Issue Type: Improvement
>Affects Versions: 1.19.0
>Reporter: Gian Merlino
>Priority: Major
>
> In particular, once for each subpredicate within the OR,
> RexSimplify.simplifyOrTerms calls {{simplify.predicates.union}} and adds the
> freshly-unioned result to {{simplify.predicates}}. The most time-consuming
> part of this seems to be {{RexUtil.predicateConstants}}, which re-examines
> each previously-added entry. This is O(N^2) in the number of subpredicates
> within the OR.
> I discovered this when someone tried to run a query with a 14,000-element IN
> filter, and planning took about 45 seconds. In Druid, we always convert INs
> to ORs, never allowing Calcite's subquery conversion to happen. This is
> because as far as native Druid queries are concerned, a huge OR is going to
> be more efficient than a join against a constant subquery.
> I'm not sure what the best way is to fix this. The only thing that comes to
> mind immediately is the "quick fix" of limiting how many OR elements
> RexSimplify might attempt to simplify at once (and potentially AND as well? I
> haven't looked into that one.)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3178) RexSimplify.simplifyOrTerms slow with large OR filters
[
https://issues.apache.org/jira/browse/CALCITE-3178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16879895#comment-16879895
]
Gian Merlino commented on CALCITE-3178:
---
[~vladimirsitnikov] No, I hadn't seen it. Do you think that overhaul would
contain a "proper fix" for this issue?
> RexSimplify.simplifyOrTerms slow with large OR filters
> --
>
> Key: CALCITE-3178
> URL: https://issues.apache.org/jira/browse/CALCITE-3178
> Project: Calcite
> Issue Type: Improvement
>Affects Versions: 1.19.0
>Reporter: Gian Merlino
>Priority: Major
>
> In particular, once for each subpredicate within the OR,
> RexSimplify.simplifyOrTerms calls {{simplify.predicates.union}} and adds the
> freshly-unioned result to {{simplify.predicates}}. The most time-consuming
> part of this seems to be {{RexUtil.predicateConstants}}, which re-examines
> each previously-added entry. This is O(N^2) in the number of subpredicates
> within the OR.
> I discovered this when someone tried to run a query with a 14,000-element IN
> filter, and planning took about 45 seconds. In Druid, we always convert INs
> to ORs, never allowing Calcite's subquery conversion to happen. This is
> because as far as native Druid queries are concerned, a huge OR is going to
> be more efficient than a join against a constant subquery.
> I'm not sure what the best way is to fix this. The only thing that comes to
> mind immediately is the "quick fix" of limiting how many OR elements
> RexSimplify might attempt to simplify at once (and potentially AND as well? I
> haven't looked into that one.)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3178) RexSimplify.simplifyOrTerms slow with large OR filters
[
https://issues.apache.org/jira/browse/CALCITE-3178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16879894#comment-16879894
]
Gian Merlino commented on CALCITE-3178:
---
This patch that does the "quick fix" of limiting OR elements sped things up a
lot for me: https://github.com/gianm/calcite/tree/limit-or-simplification. If
people feel this approach is the right one, I can add similar code for AND.
> RexSimplify.simplifyOrTerms slow with large OR filters
> --
>
> Key: CALCITE-3178
> URL: https://issues.apache.org/jira/browse/CALCITE-3178
> Project: Calcite
> Issue Type: Improvement
>Affects Versions: 1.19.0
>Reporter: Gian Merlino
>Priority: Major
>
> In particular, once for each subpredicate within the OR,
> RexSimplify.simplifyOrTerms calls {{simplify.predicates.union}} and adds the
> freshly-unioned result to {{simplify.predicates}}. The most time-consuming
> part of this seems to be {{RexUtil.predicateConstants}}, which re-examines
> each previously-added entry. This is O(N^2) in the number of subpredicates
> within the OR.
> I discovered this when someone tried to run a query with a 14,000-element IN
> filter, and planning took about 45 seconds. In Druid, we always convert INs
> to ORs, never allowing Calcite's subquery conversion to happen. This is
> because as far as native Druid queries are concerned, a huge OR is going to
> be more efficient than a join against a constant subquery.
> I'm not sure what the best way is to fix this. The only thing that comes to
> mind immediately is the "quick fix" of limiting how many OR elements
> RexSimplify might attempt to simplify at once (and potentially AND as well? I
> haven't looked into that one.)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-3178) RexSimplify.simplifyOrTerms slow with large OR filters
Gian Merlino created CALCITE-3178:
-
Summary: RexSimplify.simplifyOrTerms slow with large OR filters
Key: CALCITE-3178
URL: https://issues.apache.org/jira/browse/CALCITE-3178
Project: Calcite
Issue Type: Improvement
Affects Versions: 1.19.0
Reporter: Gian Merlino
In particular, once for each subpredicate within the OR,
RexSimplify.simplifyOrTerms calls {{simplify.predicates.union}} and adds the
freshly-unioned result to {{simplify.predicates}}. The most time-consuming part
of this seems to be {{RexUtil.predicateConstants}}, which re-examines each
previously-added entry. This is O(N^2) in the number of subpredicates within
the OR.
I discovered this when someone tried to run a query with a 14,000-element IN
filter, and planning took about 45 seconds. In Druid, we always convert INs to
ORs, never allowing Calcite's subquery conversion to happen. This is because as
far as native Druid queries are concerned, a huge OR is going to be more
efficient than a join against a constant subquery.
I'm not sure what the best way is to fix this. The only thing that comes to
mind immediately is the "quick fix" of limiting how many OR elements
RexSimplify might attempt to simplify at once (and potentially AND as well? I
haven't looked into that one.)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3144) Add rule, CaseFilteredAggregatorRule, that converts "SUM(CASE WHEN b THEN x END)" to "SUM(x) FILTER (WHERE b)"
[
https://issues.apache.org/jira/browse/CALCITE-3144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16879527#comment-16879527
]
Gian Merlino commented on CALCITE-3144:
---
On [line 234 of
AggregateCaseToFilterRule|https://github.com/apache/calcite/compare/master...julianhyde:3144-aggregate-case-to-filter#diff-cd134ae87f288527952a2726f3bd46cbR234],
case A1 will never get to run, since allowsFilter is always false -- right? Is
that intentional? It also seems like the [new check in
AggregateCall|https://github.com/apache/calcite/compare/master...julianhyde:3144-aggregate-case-to-filter#diff-46bc04fa03f7fdc22d08f0ba97ee2087R104]
would always fail when provided with a filtered aggregator. I might be missing
something.
Other than the above, the general idea seems the same as Druid's rule, so it
looks good to me from that POV. We've been using it in production for a while.
It's quite helpful to us since in Druid's query engines, a filtered aggregator
is substantially faster than evaluating a CASE function.
> Add rule, CaseFilteredAggregatorRule, that converts "SUM(CASE WHEN b THEN x
> END)" to "SUM(x) FILTER (WHERE b)"
> --
>
> Key: CALCITE-3144
> URL: https://issues.apache.org/jira/browse/CALCITE-3144
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.21.0
>
>
> Add a rule that converts "SUM(CASE WHEN b THEN x END)" to "SUM\(x) FILTER
> (WHERE b)".
> Druid added {{CaseFilteredAggregatorRule}} in
> https://github.com/apache/incubator-druid/pull/4360.
> Maybe {{AggregateCaseToFilterRule}} is a slightly better name. Or maybe this
> transform could be done in {{RelBuilder.aggregate}}, and we wouldn't need a
> rule.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3144) Add rule, CaseFilteredAggregatorRule, that converts "SUM(CASE WHEN b THEN x END)" to "SUM(x) FILTER (WHERE b)"
[
https://issues.apache.org/jira/browse/CALCITE-3144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16876495#comment-16876495
]
Gian Merlino commented on CALCITE-3144:
---
> Gian Merlino, Do you recall the reason for this line? I don't think it's
> necessary, because I think the rewrite is valid even if there are GROUPING
> SETS etc.
I don't recall, but, Druid doesn't currently support GROUPING SETS so the
thought process may have been "well, let's not worry about whether or not this
is valid for GROUPING SETS or not".
> Add rule, CaseFilteredAggregatorRule, that converts "SUM(CASE WHEN b THEN x
> END)" to "SUM(x) FILTER (WHERE b)"
> --
>
> Key: CALCITE-3144
> URL: https://issues.apache.org/jira/browse/CALCITE-3144
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> Add a rule that converts "SUM(CASE WHEN b THEN x END)" to "SUM\(x) FILTER
> (WHERE b)".
> Druid added {{CaseFilteredAggregatorRule}} in
> https://github.com/apache/incubator-druid/pull/4360.
> Maybe {{AggregateCaseToFilterRule}} is a slightly better name. Or maybe this
> transform could be done in {{RelBuilder.aggregate}}, and we wouldn't need a
> rule.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2635) getMonotonocity is slow on wide tables
[
https://issues.apache.org/jira/browse/CALCITE-2635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16658057#comment-16658057
]
Gian Merlino commented on CALCITE-2635:
---
Any suggestions on what file is appropriate for that test?
I'm a bit concerned about the ability of a test like that to verify
performance: the added overhead is only about 2 seconds, which is probably
within normal variation across test environments. Even trying both ways (a big
table and small table) and comparing in the same environment seems liable to
false failures.
> getMonotonocity is slow on wide tables
> --
>
> Key: CALCITE-2635
> URL: https://issues.apache.org/jira/browse/CALCITE-2635
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Gian Merlino
>Priority: Major
> Labels: performance
>
> RelOptTableImpl's getMonotonocity does an indexOf on
> {{rowType.getFieldNames()}}, which is O(N) in the number of fields.
> IdentifierNamespace calls getMonotonicity once for every field in the table
> namespace, so it becomes O(N^2) in the number of fields. We observed 2-4
> second query planning times with a table that had 18,000 columns, reduced to
> about 150ms after patching getMonotonicity to be O(1) in the number of fields.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (CALCITE-2635) getMonotonocity is slow on wide tables
[
https://issues.apache.org/jira/browse/CALCITE-2635?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-2635:
--
External issue ID: https://github.com/apache/calcite/pull/891
> getMonotonocity is slow on wide tables
> --
>
> Key: CALCITE-2635
> URL: https://issues.apache.org/jira/browse/CALCITE-2635
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Gian Merlino
>Priority: Major
> Labels: performance
>
> RelOptTableImpl's getMonotonocity does an indexOf on
> {{rowType.getFieldNames()}}, which is O(N) in the number of fields.
> IdentifierNamespace calls getMonotonicity once for every field in the table
> namespace, so it becomes O(N^2) in the number of fields. We observed 2-4
> second query planning times with a table that had 18,000 columns, reduced to
> about 150ms after patching getMonotonicity to be O(1) in the number of fields.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (CALCITE-2635) getMonotonocity is slow on wide tables
Gian Merlino created CALCITE-2635:
-
Summary: getMonotonocity is slow on wide tables
Key: CALCITE-2635
URL: https://issues.apache.org/jira/browse/CALCITE-2635
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Gian Merlino
Assignee: Gian Merlino
RelOptTableImpl's getMonotonocity does an indexOf on
{{rowType.getFieldNames()}}, which is O(N) in the number of fields.
IdentifierNamespace calls getMonotonicity once for every field in the table
namespace, so it becomes O(N^2) in the number of fields. We observed 2-4 second
query planning times with a table that had 18,000 columns, reduced to about
150ms after patching getMonotonicity to be O(1) in the number of fields.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2035) Add APPROX_COUNT_DISTINCT aggregate function
[
https://issues.apache.org/jira/browse/CALCITE-2035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16242659#comment-16242659
]
Gian Merlino commented on CALCITE-2035:
---
[~julianhyde], I just took a look and only had a couple of small comments. The
syntax and behavior contract look good to me.
> Add APPROX_COUNT_DISTINCT aggregate function
>
>
> Key: CALCITE-2035
> URL: https://issues.apache.org/jira/browse/CALCITE-2035
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>
> Add {{APPROX_COUNT_DISTINCT}} aggregate function. The effect of
> {{APPROX_COUNT_DISTINCT(args)}} is the same as {{COUNT(DISTINCT args)}} but
> the planner may generate approximate results (e.g. by using HyperLogLog).
> Note "may" not "must", above: the planner may choose a plan that returns
> exact results.
> This is a step towards CALCITE-1588, which would allow an {{APPROXIMATE}}
> clause and specify in more detail the degree of approximation allowed.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-1910) NPE on filtered aggregators using "IN"
[
https://issues.apache.org/jira/browse/CALCITE-1910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16207796#comment-16207796
]
Gian Merlino commented on CALCITE-1910:
---
Thanks for the review [~michaelmior]!
Fixed in
https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=188c8020d4e68c0a3180265b07949aeb8830ff1b.
> NPE on filtered aggregators using "IN"
> --
>
> Key: CALCITE-1910
> URL: https://issues.apache.org/jira/browse/CALCITE-1910
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> This test causes an NPE in SqlToRelConverter. I haven't figured out why, but
> I guess it has something to do with not calling "replaceSubQueries" on the
> second arg of the FILTER SqlNode.
> {code}
> @Test public void testAggFilterWithIn() {
> final String sql = "select\n"
> + " deptno, sum(sal * 2) filter (where empno not in (1, 2)),
> count(*)\n"
> + "from emp\n"
> + "group by deptno";
> sql(sql).ok();
> }
> {code}
> The stack trace is:
> {code}
> java.lang.NullPointerException
> at
> com.google.common.base.Preconditions.checkNotNull(Preconditions.java:212)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4426)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.translateAgg(SqlToRelConverter.java:4888)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4819)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
> at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:137)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4795)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
> at org.apache.calcite.sql.SqlNodeList.accept(SqlNodeList.java:153)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.createAggImpl(SqlToRelConverter.java:2715)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertAgg(SqlToRelConverter.java:2656)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:658)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:620)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3066)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:556)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:574)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:682)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:2676)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.ok(SqlToRelConverterTest.java:2668)
> at
> org.apache.calcite.test.SqlToRelConverterTest.testAggFilterWithIn(SqlToRelConverterTest.java:477)
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Resolved] (CALCITE-1910) NPE on filtered aggregators using "IN"
[
https://issues.apache.org/jira/browse/CALCITE-1910?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-1910.
---
Resolution: Fixed
Assignee: Gian Merlino (was: Julian Hyde)
> NPE on filtered aggregators using "IN"
> --
>
> Key: CALCITE-1910
> URL: https://issues.apache.org/jira/browse/CALCITE-1910
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Gian Merlino
>Assignee: Gian Merlino
>
> This test causes an NPE in SqlToRelConverter. I haven't figured out why, but
> I guess it has something to do with not calling "replaceSubQueries" on the
> second arg of the FILTER SqlNode.
> {code}
> @Test public void testAggFilterWithIn() {
> final String sql = "select\n"
> + " deptno, sum(sal * 2) filter (where empno not in (1, 2)),
> count(*)\n"
> + "from emp\n"
> + "group by deptno";
> sql(sql).ok();
> }
> {code}
> The stack trace is:
> {code}
> java.lang.NullPointerException
> at
> com.google.common.base.Preconditions.checkNotNull(Preconditions.java:212)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4426)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.translateAgg(SqlToRelConverter.java:4888)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4819)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
> at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:137)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4795)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
> at org.apache.calcite.sql.SqlNodeList.accept(SqlNodeList.java:153)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.createAggImpl(SqlToRelConverter.java:2715)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertAgg(SqlToRelConverter.java:2656)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:658)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:620)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3066)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:556)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:574)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:682)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:2676)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.ok(SqlToRelConverterTest.java:2668)
> at
> org.apache.calcite.test.SqlToRelConverterTest.testAggFilterWithIn(SqlToRelConverterTest.java:477)
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-1910) NPE on filtered aggregators using "IN"
[
https://issues.apache.org/jira/browse/CALCITE-1910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16206822#comment-16206822
]
Gian Merlino commented on CALCITE-1910:
---
I wrote a patch and test case in https://github.com/apache/calcite/pull/548.
I'm not sure if this is the "right" way to solve it or not, but it seems to
work. Is anyone able to sanity check the patch?
> NPE on filtered aggregators using "IN"
> --
>
> Key: CALCITE-1910
> URL: https://issues.apache.org/jira/browse/CALCITE-1910
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> This test causes an NPE in SqlToRelConverter. I haven't figured out why, but
> I guess it has something to do with not calling "replaceSubQueries" on the
> second arg of the FILTER SqlNode.
> {code}
> @Test public void testAggFilterWithIn() {
> final String sql = "select\n"
> + " deptno, sum(sal * 2) filter (where empno not in (1, 2)),
> count(*)\n"
> + "from emp\n"
> + "group by deptno";
> sql(sql).ok();
> }
> {code}
> The stack trace is:
> {code}
> java.lang.NullPointerException
> at
> com.google.common.base.Preconditions.checkNotNull(Preconditions.java:212)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4426)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.translateAgg(SqlToRelConverter.java:4888)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4819)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
> at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:137)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4795)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
> at org.apache.calcite.sql.SqlNodeList.accept(SqlNodeList.java:153)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.createAggImpl(SqlToRelConverter.java:2715)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertAgg(SqlToRelConverter.java:2656)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:658)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:620)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3066)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:556)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:574)
> at
> org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:682)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:2676)
> at
> org.apache.calcite.test.SqlToRelConverterTest$Sql.ok(SqlToRelConverterTest.java:2668)
> at
> org.apache.calcite.test.SqlToRelConverterTest.testAggFilterWithIn(SqlToRelConverterTest.java:477)
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16202387#comment-16202387
]
Gian Merlino commented on CALCITE-2007:
---
Got it, thanks for the tips.
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Gian Merlino
> Fix For: 1.15.0
>
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Resolved] (CALCITE-2008) Fix braces in TRIM signature
[
https://issues.apache.org/jira/browse/CALCITE-2008?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-2008.
---
Resolution: Fixed
> Fix braces in TRIM signature
>
>
> Key: CALCITE-2008
> URL: https://issues.apache.org/jira/browse/CALCITE-2008
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Gian Merlino
>Assignee: Gian Merlino
> Fix For: 1.15.0
>
>
> The signature says
> {noformat}
> [BOTH|LEADING|TRAILING}
> {noformat}
> but it should be
> {noformat}
> [BOTH|LEADING|TRAILING]
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-2008) Fix braces in TRIM signature
[
https://issues.apache.org/jira/browse/CALCITE-2008?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16202050#comment-16202050
]
Gian Merlino commented on CALCITE-2008:
---
Fixed in
https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=6c435a03e83eac6c0d9f13b87ef22ad9f93df714
> Fix braces in TRIM signature
>
>
> Key: CALCITE-2008
> URL: https://issues.apache.org/jira/browse/CALCITE-2008
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Gian Merlino
>Assignee: Gian Merlino
> Fix For: 1.15.0
>
>
> The signature says
> {noformat}
> [BOTH|LEADING|TRAILING}
> {noformat}
> but it should be
> {noformat}
> [BOTH|LEADING|TRAILING]
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Created] (CALCITE-2008) Fix braces in TRIM signature
Gian Merlino created CALCITE-2008:
-
Summary: Fix braces in TRIM signature
Key: CALCITE-2008
URL: https://issues.apache.org/jira/browse/CALCITE-2008
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Gian Merlino
Assignee: Gian Merlino
Fix For: 1.15.0
The signature says
{noformat}
[BOTH|LEADING|TRAILING}
{noformat}
but it should be
{noformat}
[BOTH|LEADING|TRAILING]
{noformat}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Updated] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-2007:
--
Fix Version/s: 1.15.0
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
> Fix For: 1.15.0
>
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Resolved] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-2007.
---
Resolution: Fixed
Assignee: Gian Merlino (was: Julian Hyde)
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Gian Merlino
> Fix For: 1.15.0
>
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16201406#comment-16201406
]
Gian Merlino commented on CALCITE-2007:
---
Fixed in
https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=06c18ca74df2d4bf20a5dccef11d424ab28c97d7.
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16201404#comment-16201404
]
Gian Merlino commented on CALCITE-2007:
---
Thanks for the review Julian. I'll make those changes and then commit the patch.
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Comment Edited] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16201254#comment-16201254
]
Gian Merlino edited comment on CALCITE-2007 at 10/12/17 12:11 AM:
--
[~julianhyde] last time I raised a PR you asked me if I could commit it
directly. I haven't done that before, so I'm wondering, what is the process? Is
it important to get someone else to review the code first or is it OK to commit
unilaterally and then just update the JIRA?
Sorry if this is answered in a doc that I didn't see. Thanks!
was (Author: gian):
[~julianhyde] last time I raised a PR you asked me if I could commit it
directly. I haven't done that before, so I'm wondering, what is the process?
Should I click merge the PR? Or commit using the CLI git client and then close
the PR without hitting the merge button? I can see by looking at other commits
that it's important to include the JIRA number in the git commit message, but
is it important to include the PR number? Or was raising a PR pointless if I
was just going to commit it myself?
And finally is it important to get someone else to review the code first or is
it OK to commit unilaterally?
Sorry if these questions are answered in a doc that I didn't see. Thanks!
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16201254#comment-16201254
]
Gian Merlino commented on CALCITE-2007:
---
[~julianhyde] last time I raised a PR you asked me if I could commit it
directly. I haven't done that before, so I'm wondering, what is the process?
Should I click merge the PR? Or commit using the CLI git client and then close
the PR without hitting the merge button? I can see by looking at other commits
that it's important to include the JIRA number in the git commit message, but
is it important to include the PR number? Or was raising a PR pointless if I
was just going to commit it myself?
And finally is it important to get someone else to review the code first or is
it OK to commit unilaterally?
Sorry if these questions are answered in a doc that I didn't see. Thanks!
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Created] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
Gian Merlino created CALCITE-2007:
-
Summary: RexSimplify incorrectly simplifies AND bounds
Key: CALCITE-2007
URL: https://issues.apache.org/jira/browse/CALCITE-2007
Project: Calcite
Issue Type: Bug
Components: core
Affects Versions: 1.14.0
Reporter: Gian Merlino
Assignee: Julian Hyde
For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
3}}.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Updated] (CALCITE-2007) RexSimplify incorrectly simplifies AND bounds
[
https://issues.apache.org/jira/browse/CALCITE-2007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-2007:
--
Description: For example, RexSimplify incorrectly simplifies {{1 < x AND x
< 3}} to {{x < 3}}. Patch in linked PR. (was: For example, RexSimplify
incorrectly simplifies {{1 < x AND x < 3}} to {{x < 3}}.)
> RexSimplify incorrectly simplifies AND bounds
> -
>
> Key: CALCITE-2007
> URL: https://issues.apache.org/jira/browse/CALCITE-2007
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.14.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> For example, RexSimplify incorrectly simplifies {{1 < x AND x < 3}} to {{x <
> 3}}. Patch in linked PR.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-1588) Add SQL syntax to allow approximate LIMIT and distinct-COUNT
[
https://issues.apache.org/jira/browse/CALCITE-1588?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16107560#comment-16107560
]
Gian Merlino commented on CALCITE-1588:
---
In Druid at least, you don't get to specify the accuracy you want. HLL in
general does have some parameters but Druid uses hard-coded constants for them.
> Add SQL syntax to allow approximate LIMIT and distinct-COUNT
>
>
> Key: CALCITE-1588
> URL: https://issues.apache.org/jira/browse/CALCITE-1588
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>
> Add SQL syntax to allow approximate LIMIT and distinct-COUNT. These will set
> the properties specified in CALCITE-1587. By default the properties are
> false, so the query will return exact results.
> Exact syntax is to be decided. It could be at the top of the query (therefore
> affecting every LIMIT or aggregate in the query) or it could be more
> localized (e.g. {{COUNT(DISTINCT customerId) APPROXIMATE (WITHIN 10
> PERCENT)}}).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Created] (CALCITE-1910) NPE on filtered aggregators using "IN"
Gian Merlino created CALCITE-1910:
-
Summary: NPE on filtered aggregators using "IN"
Key: CALCITE-1910
URL: https://issues.apache.org/jira/browse/CALCITE-1910
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Gian Merlino
Assignee: Julian Hyde
This test causes an NPE in SqlToRelConverter. I haven't figured out why, but I
guess it has something to do with not calling "replaceSubQueries" on the second
arg of the FILTER SqlNode.
{code}
@Test public void testAggFilterWithIn() {
final String sql = "select\n"
+ " deptno, sum(sal * 2) filter (where empno not in (1, 2)),
count(*)\n"
+ "from emp\n"
+ "group by deptno";
sql(sql).ok();
}
{code}
The stack trace is:
{code}
java.lang.NullPointerException
at
com.google.common.base.Preconditions.checkNotNull(Preconditions.java:212)
at
org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4426)
at
org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.translateAgg(SqlToRelConverter.java:4888)
at
org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4819)
at
org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:137)
at
org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4795)
at
org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4663)
at org.apache.calcite.sql.SqlNodeList.accept(SqlNodeList.java:153)
at
org.apache.calcite.sql2rel.SqlToRelConverter.createAggImpl(SqlToRelConverter.java:2715)
at
org.apache.calcite.sql2rel.SqlToRelConverter.convertAgg(SqlToRelConverter.java:2656)
at
org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:658)
at
org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:620)
at
org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:3066)
at
org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:556)
at
org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:574)
at
org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:682)
at
org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:2676)
at
org.apache.calcite.test.SqlToRelConverterTest$Sql.ok(SqlToRelConverterTest.java:2668)
at
org.apache.calcite.test.SqlToRelConverterTest.testAggFilterWithIn(SqlToRelConverterTest.java:477)
{code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Resolved] (CALCITE-1799) OR IN Subquery conversion wrong
[
https://issues.apache.org/jira/browse/CALCITE-1799?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino resolved CALCITE-1799.
---
Resolution: Won't Fix
Marked "won't fix" (for the old subquery expander) based on prior comments.
> OR IN Subquery conversion wrong
> ---
>
> Key: CALCITE-1799
> URL: https://issues.apache.org/jira/browse/CALCITE-1799
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.12.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
> Labels: sub-query
>
> This query:
> {code}
> select * from emp where deptno = 10 or deptno in (
> select dept.deptno from dept where deptno < 5)
> {code}
> Is converted to this by SqlToRelConverter:
> {code}
> LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
> SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
> LogicalFilter(condition=[OR(=($7, 10), true)])
> LogicalJoin(condition=[=($7, $9)], joinType=[inner])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalAggregate(group=[{0}])
> LogicalProject(DEPTNO=[$0])
> LogicalFilter(condition=[<($0, 5)])
> LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
> {code}
> But that's not right. {code}LogicalFilter(condition=[OR(=($7, 10),
> true)]){code} is always true and is in the wrong place anyway (it's applied
> after the inner join, where all the deptno = 10 records have already been
> removed).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-1799) OR IN Subquery conversion wrong
[
https://issues.apache.org/jira/browse/CALCITE-1799?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16075595#comment-16075595
]
Gian Merlino commented on CALCITE-1799:
---
> Let's mark it as fixed by the other bug (do you recall the bug number?); if
> people choose to use the (broken) backwards compatible behavior that's their
> choice.
Which bug do you mean? As far as I know there isn't one, this is just something
that works through one code path but not through another.
> I think you mean
> https://github.com/gianm/calcite/tree/planner-configure-sqlToRelConverter,
> not CALCITE-1873. Can you make a PR for that (or just commit it); it would be
> useful, and in fact someone was asking for something similar on the dev list
> a day or two ago
Sorry, I meant CALCITE-1874. There is already a PR on that one. Since I'm still
new I'd prefer someone reviewing the patch before just committing it, if you
don't mind.
> OR IN Subquery conversion wrong
> ---
>
> Key: CALCITE-1799
> URL: https://issues.apache.org/jira/browse/CALCITE-1799
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.12.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
> Labels: sub-query
>
> This query:
> {code}
> select * from emp where deptno = 10 or deptno in (
> select dept.deptno from dept where deptno < 5)
> {code}
> Is converted to this by SqlToRelConverter:
> {code}
> LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
> SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
> LogicalFilter(condition=[OR(=($7, 10), true)])
> LogicalJoin(condition=[=($7, $9)], joinType=[inner])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalAggregate(group=[{0}])
> LogicalProject(DEPTNO=[$0])
> LogicalFilter(condition=[<($0, 5)])
> LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
> {code}
> But that's not right. {code}LogicalFilter(condition=[OR(=($7, 10),
> true)]){code} is always true and is in the wrong place anyway (it's applied
> after the inner join, where all the deptno = 10 records have already been
> removed).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Updated] (CALCITE-1874) Configurable SqlToRelConverter.Config in Frameworks
[ https://issues.apache.org/jira/browse/CALCITE-1874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gian Merlino updated CALCITE-1874: -- Issue Type: Improvement (was: Bug) > Configurable SqlToRelConverter.Config in Frameworks > --- > > Key: CALCITE-1874 > URL: https://issues.apache.org/jira/browse/CALCITE-1874 > Project: Calcite > Issue Type: Improvement > Components: core >Reporter: Gian Merlino >Assignee: Julian Hyde > > Like SqlParser.Config, it's useful to be able to configure the > SqlToRelConverter.Config. See also CALCITE-1799. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (CALCITE-1799) OR IN Subquery conversion wrong
[
https://issues.apache.org/jira/browse/CALCITE-1799?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16075537#comment-16075537
]
Gian Merlino commented on CALCITE-1799:
---
[~julianhyde] do you think it makes sense to keep this bug open (since it's
still a bug in the old code) or to close it (since the new code seems to work
fine)?
> OR IN Subquery conversion wrong
> ---
>
> Key: CALCITE-1799
> URL: https://issues.apache.org/jira/browse/CALCITE-1799
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.12.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
> Labels: sub-query
>
> This query:
> {code}
> select * from emp where deptno = 10 or deptno in (
> select dept.deptno from dept where deptno < 5)
> {code}
> Is converted to this by SqlToRelConverter:
> {code}
> LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
> SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
> LogicalFilter(condition=[OR(=($7, 10), true)])
> LogicalJoin(condition=[=($7, $9)], joinType=[inner])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalAggregate(group=[{0}])
> LogicalProject(DEPTNO=[$0])
> LogicalFilter(condition=[<($0, 5)])
> LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
> {code}
> But that's not right. {code}LogicalFilter(condition=[OR(=($7, 10),
> true)]){code} is always true and is in the wrong place anyway (it's applied
> after the inner join, where all the deptno = 10 records have already been
> removed).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-1874) Configurable SqlToRelConverter.Config in Frameworks
[ https://issues.apache.org/jira/browse/CALCITE-1874?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16075538#comment-16075538 ] Gian Merlino commented on CALCITE-1874: --- Patch in https://github.com/apache/calcite/pull/492. > Configurable SqlToRelConverter.Config in Frameworks > --- > > Key: CALCITE-1874 > URL: https://issues.apache.org/jira/browse/CALCITE-1874 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Gian Merlino >Assignee: Julian Hyde > > Like SqlParser.Config, it's useful to be able to configure the > SqlToRelConverter.Config. See also CALCITE-1799. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Created] (CALCITE-1874) Configurable SqlToRelConverter.Config in Frameworks
Gian Merlino created CALCITE-1874: - Summary: Configurable SqlToRelConverter.Config in Frameworks Key: CALCITE-1874 URL: https://issues.apache.org/jira/browse/CALCITE-1874 Project: Calcite Issue Type: Bug Components: core Reporter: Gian Merlino Assignee: Julian Hyde Like SqlParser.Config, it's useful to be able to configure the SqlToRelConverter.Config. See also CALCITE-1799. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (CALCITE-1799) OR IN Subquery conversion wrong
[
https://issues.apache.org/jira/browse/CALCITE-1799?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16075533#comment-16075533
]
Gian Merlino commented on CALCITE-1799:
---
[~julianhyde], I finally got around to testing this, and the query plan is ok
when disabling subquery expansion in SqlToRelConverter. However, to actually do
this from within my code, I had to add a method to Frameworks to accept a
SqlToRelConverter Config; that's in CALCITE-1873.
> OR IN Subquery conversion wrong
> ---
>
> Key: CALCITE-1799
> URL: https://issues.apache.org/jira/browse/CALCITE-1799
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.12.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
> Labels: sub-query
>
> This query:
> {code}
> select * from emp where deptno = 10 or deptno in (
> select dept.deptno from dept where deptno < 5)
> {code}
> Is converted to this by SqlToRelConverter:
> {code}
> LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
> SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
> LogicalFilter(condition=[OR(=($7, 10), true)])
> LogicalJoin(condition=[=($7, $9)], joinType=[inner])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalAggregate(group=[{0}])
> LogicalProject(DEPTNO=[$0])
> LogicalFilter(condition=[<($0, 5)])
> LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
> {code}
> But that's not right. {code}LogicalFilter(condition=[OR(=($7, 10),
> true)]){code} is always true and is in the wrong place anyway (it's applied
> after the inner join, where all the deptno = 10 records have already been
> removed).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (CALCITE-1873) Validator errors when grouping by ordinals that refer to aliases
[ https://issues.apache.org/jira/browse/CALCITE-1873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16075263#comment-16075263 ] Gian Merlino commented on CALCITE-1873: --- More details & patch in the linked PR. For some reason, not clear to me at this point, the error only happens if you are also ordering by an ordinal (like "group by 1 order by 1"). > Validator errors when grouping by ordinals that refer to aliases > > > Key: CALCITE-1873 > URL: https://issues.apache.org/jira/browse/CALCITE-1873 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Gian Merlino >Assignee: Julian Hyde > > Patch in https://github.com/apache/calcite/pull/490. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Created] (CALCITE-1873) Validator errors when grouping by ordinals that refer to aliases
Gian Merlino created CALCITE-1873: - Summary: Validator errors when grouping by ordinals that refer to aliases Key: CALCITE-1873 URL: https://issues.apache.org/jira/browse/CALCITE-1873 Project: Calcite Issue Type: Bug Components: core Reporter: Gian Merlino Assignee: Julian Hyde -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (CALCITE-1873) Validator errors when grouping by ordinals that refer to aliases
[ https://issues.apache.org/jira/browse/CALCITE-1873?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gian Merlino updated CALCITE-1873: -- Description: Patch in https://github.com/apache/calcite/pull/490. > Validator errors when grouping by ordinals that refer to aliases > > > Key: CALCITE-1873 > URL: https://issues.apache.org/jira/browse/CALCITE-1873 > Project: Calcite > Issue Type: Bug > Components: core >Reporter: Gian Merlino >Assignee: Julian Hyde > > Patch in https://github.com/apache/calcite/pull/490. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Created] (CALCITE-1799) OR IN Subquery conversion wrong
Gian Merlino created CALCITE-1799:
-
Summary: OR IN Subquery conversion wrong
Key: CALCITE-1799
URL: https://issues.apache.org/jira/browse/CALCITE-1799
Project: Calcite
Issue Type: Bug
Components: core
Affects Versions: 1.12.0
Reporter: Gian Merlino
Assignee: Julian Hyde
This query:
{code}
select * from emp where deptno = 10 or deptno in (
select dept.deptno from dept where deptno < 5)
{code}
Is converted to this by SqlToRelConverter:
{code}
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[OR(=($7, 10), true)])
LogicalJoin(condition=[=($7, $9)], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalAggregate(group=[{0}])
LogicalProject(DEPTNO=[$0])
LogicalFilter(condition=[<($0, 5)])
LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
{code}
But that's not right. {code}LogicalFilter(condition=[OR(=($7, 10),
true)]){code} is always true and is in the wrong place anyway (it's applied
after the inner join, where all the deptno = 10 records have already been
removed).
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1719) Collapse nested limits
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15982376#comment-15982376
]
Gian Merlino commented on CALCITE-1719:
---
I just re-titled and updated the description -- hope it's accurate now.
> Collapse nested limits
> --
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Originally proposed to be fixed by an optimizer rule, but for reasons later
> on in the jira, it seems it's better to do this in SqlToRelConverter instead.
> Original patch was in https://github.com/apache/calcite/pull/410, now closed.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (CALCITE-1719) Collapse nested limits
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-1719:
--
Description:
Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
query might have an order by or limit.
Originally proposed to be fixed by an optimizer rule, but for reasons later on
in the jira, it seems it's better to do this in SqlToRelConverter instead.
Original patch was in https://github.com/apache/calcite/pull/410, now closed.
was:
Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
query might have an order by or limit.
-Patch in https://github.com/apache/calcite/pull/410 x-
> Collapse nested limits
> --
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Originally proposed to be fixed by an optimizer rule, but for reasons later
> on in the jira, it seems it's better to do this in SqlToRelConverter instead.
> Original patch was in https://github.com/apache/calcite/pull/410, now closed.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (CALCITE-1719) Collapse nested limits
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-1719:
--
Summary: Collapse nested limits (was: SortCollapseRule for collapsing
nested sorts)
> Collapse nested limits
> --
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (CALCITE-1719) Collapse nested limits
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gian Merlino updated CALCITE-1719:
--
Description:
Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
query might have an order by or limit.
-Patch in https://github.com/apache/calcite/pull/410 x-
was:
Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
query might have an order by or limit.
Patch in https://github.com/apache/calcite/pull/410.
> Collapse nested limits
> --
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> -Patch in https://github.com/apache/calcite/pull/410 x-
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15982268#comment-15982268
]
Gian Merlino commented on CALCITE-1719:
---
[~julianhyde], it sounds like the approach here is a non-starter for my
problem, and changing SqlToRelConverter is the way to go for doing the thing I
wanted to do. Should I close this bug then and open a new one for changes to
SqlToRelConverter? Or re-title this one?
> SortCollapseRule for collapsing nested sorts
>
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Comment Edited] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15962348#comment-15962348
]
Gian Merlino edited comment on CALCITE-1719 at 4/10/17 2:54 AM:
[~julianhyde], the motivation for this rule is to make it possible for a query
tool to apply a limit on a user-entered query it doesn't understand, by
wrapping it in {{SELECT * FROM ( whatever ) LIMIT 10}}. That allows a query
tool to treat the user-entered query as an opaque string. The changes you're
suggesting would make a rule that is too narrow to be useful for this case.
So I have two lines of questions that I hope you can help with.
1. My reasoning with this rule is that it makes sense for SQL like {{SELECT *
FROM something LIMIT 10}} or even {{SELECT * FROM something}} (without the
limit) to yield a result that retains the same collation of the relation
{{something}}, whether that's a table or a subquery or what have you. Is that
line of reasoning bad?
2. Assuming this approach is a non starter, do you have any other ideas for the
original problem? (A query tool that wants to apply limits to queries it can't
parse.) It's possible for the tool to execute the original query and then stop
reading after a certain number of rows, but this solution isn't very satisfying
since it means the planner won't be aware of the limit and can't use it to
optimize execution.
was (Author: gian):
[~julianhyde], the motivation for this rule is to make it possible for a query
tool to apply a limit on a user-entered query it doesn't understand, by
wrapping it in {{SELECT * FROM ( whatever ) LIMIT 10}}. That allows a query
tool to treat the user-entered query as an opaque string. The changes you're
suggesting would make a rule that is too narrow to be useful for this case.
So I have two lines of questions that I hope you can help with.
1. My reasoning with this rule is that it makes sense for SQL like {{SELECT *
FROM something LIMIT 10}} or even {{SELECT * FROM something}} (without the
limit) to yield a result that retains the same collation of the relation
{{something}}, whether that's a table or a subquery or what have you. In other
words I think it makes sense for a Sort with no sort key to yield a result with
the same collation as its input. Is that line of reasoning bad?
2. Assuming this approach is a non starter, do you have any other ideas for the
original problem? (A query tool that wants to apply limits to queries it can't
parse.) It's possible for the tool to execute the original query and then stop
reading after a certain number of rows, but this solution isn't very satisfying
since it means the planner won't be aware of the limit and can't use it to
optimize execution.
> SortCollapseRule for collapsing nested sorts
>
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15962348#comment-15962348
]
Gian Merlino commented on CALCITE-1719:
---
[~julianhyde], the motivation for this rule is to make it possible for a query
tool to apply a limit on a user-entered query it doesn't understand, by
wrapping it in {{SELECT * FROM ( whatever ) LIMIT 10}}. That allows a query
tool to treat the user-entered query as an opaque string. The changes you're
suggesting would make a rule that is too narrow to be useful for this case.
So I have two lines of questions that I hope you can help with.
1. My reasoning with this rule is that it makes sense for SQL like {{SELECT *
FROM something LIMIT 10}} or even {{SELECT * FROM something}} (without the
limit) to yield a result that retains the same collation of the relation
{{something}}, whether that's a table or a subquery or what have you. In other
words I think it makes sense for a Sort with no sort key to yield a result with
the same collation as its input. Is that line of reasoning bad?
2. Assuming this approach is a non starter, do you have any other ideas for the
original problem? (A query tool that wants to apply limits to queries it can't
parse.) It's possible for the tool to execute the original query and then stop
reading after a certain number of rows, but this solution isn't very satisfying
since it means the planner won't be aware of the limit and can't use it to
optimize execution.
> SortCollapseRule for collapsing nested sorts
>
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15937124#comment-15937124
]
Gian Merlino commented on CALCITE-1719:
---
Got it, that makes sense. This rule should be fine then, since it's not
assuming anything about the order of rows coming out of the inner Sort. It just
merges two layers of limit/offset.
I get the thing about the style. Fwiw, in Druid we have an IntelliJ config and
a checkstyle verifier as well. Neither one catches everything we care about,
but together they catch most things. Contributors don't need to use IntelliJ,
but the ones that do will get fewer style changes requested in code review.
> SortCollapseRule for collapsing nested sorts
>
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15937111#comment-15937111
]
Gian Merlino commented on CALCITE-1719:
---
I updated the patch with the test and style changes.
> SortCollapseRule for collapsing nested sorts
>
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
[
https://issues.apache.org/jira/browse/CALCITE-1719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15937072#comment-15937072
]
Gian Merlino commented on CALCITE-1719:
---
The rule shouldn't care about the inner sort order at all, it's just looking at
the limits and offsets. The rule also won't fire if the outer sort has any
collations associated with it, since in that case, the two sorts aren't in
general going to be collapsible.
Out of curiosity what do you mean by "it's dangerous to assume that Sort
produces sorted output"? Surely if a Sort has a collation associated with it
then it should produce sorted output? If not, then the limit and offset would
have potentially wrong behavior.
I'll adjust the test.
Is there an IntelliJ code style config I can import that matches the Calcite
code style? The code passed checkstyle but it sounds like there's some things
that checkstyle isn't catching.
> SortCollapseRule for collapsing nested sorts
>
>
> Key: CALCITE-1719
> URL: https://issues.apache.org/jira/browse/CALCITE-1719
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Reporter: Gian Merlino
>Assignee: Julian Hyde
>
> Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
> query might have an order by or limit.
> Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1718) Incorrect data type for context in Druid adapter
[
https://issues.apache.org/jira/browse/CALCITE-1718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15937016#comment-15937016
]
Gian Merlino commented on CALCITE-1718:
---
There isn't really a preferred form, either is supposed to work equally well.
I just tried this query on Druid 0.9.2 and it worked with both {{true}} and
{{"true"}}. Is it possible you have some older versions of Druid running on
some of your nodes?
> Incorrect data type for context in Druid adapter
>
>
> Key: CALCITE-1718
> URL: https://issues.apache.org/jira/browse/CALCITE-1718
> Project: Calcite
> Issue Type: Bug
> Components: druid
>Affects Versions: 1.11.0
>Reporter: Michael Mior
>Assignee: Michael Mior
>
> The context {{skipEmptyBuckets}} added in
> [CALCITE-1589|https://issues.apache.org/jira/browse/CALCITE-1589] was given
> the boolean value {{true}}. The [Druid
> documentation|http://druid.io/docs/latest/querying/timeseriesquery.html]
> shows that this should be the string {{"true"}}. I get a test failure on
> Druid 0.9.2 because of this discrepancy.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Created] (CALCITE-1719) SortCollapseRule for collapsing nested sorts
Gian Merlino created CALCITE-1719:
-
Summary: SortCollapseRule for collapsing nested sorts
Key: CALCITE-1719
URL: https://issues.apache.org/jira/browse/CALCITE-1719
Project: Calcite
Issue Type: Improvement
Components: core
Reporter: Gian Merlino
Assignee: Julian Hyde
Useful for subqueries like {{SELECT * FROM (...) LIMIT X}}, where the inner
query might have an order by or limit.
Patch in https://github.com/apache/calcite/pull/410.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1718) Incorrect data type for context in Druid adapter
[
https://issues.apache.org/jira/browse/CALCITE-1718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15935764#comment-15935764
]
Gian Merlino commented on CALCITE-1718:
---
[~michaelmior], what error do you get? Druid 0.9.1.1+ should be able to handle
either {{true}} or {{"true"}}.
> Incorrect data type for context in Druid adapter
>
>
> Key: CALCITE-1718
> URL: https://issues.apache.org/jira/browse/CALCITE-1718
> Project: Calcite
> Issue Type: Bug
> Components: druid
>Affects Versions: 1.11.0
>Reporter: Michael Mior
>Assignee: Michael Mior
>
> The context {{skipEmptyBuckets}} added in
> [CALCITE-1589|https://issues.apache.org/jira/browse/CALCITE-1589] was given
> the boolean value {{true}}. The [Druid
> documentation|http://druid.io/docs/latest/querying/timeseriesquery.html]
> shows that this should be the string {{"true"}}. I get a test failure on
> Druid 0.9.2 because of this discrepancy.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1714) Do not push group by on druid metrics fields
[ https://issues.apache.org/jira/browse/CALCITE-1714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15934102#comment-15934102 ] Gian Merlino commented on CALCITE-1714: --- (master already does) > Do not push group by on druid metrics fields > > > Key: CALCITE-1714 > URL: https://issues.apache.org/jira/browse/CALCITE-1714 > Project: Calcite > Issue Type: Bug > Components: druid >Affects Versions: 1.12.0 >Reporter: slim bouguerra >Assignee: Julian Hyde > > Druid does not support grouping by metrics, hence we can not push the group > by to druid. Instead we should generate a select query. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (CALCITE-1714) Do not push group by on druid metrics fields
[ https://issues.apache.org/jira/browse/CALCITE-1714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15934101#comment-15934101 ] Gian Merlino commented on CALCITE-1714: --- Druid 0.10.0 will support grouping by numeric columns, fwiw. > Do not push group by on druid metrics fields > > > Key: CALCITE-1714 > URL: https://issues.apache.org/jira/browse/CALCITE-1714 > Project: Calcite > Issue Type: Bug > Components: druid >Affects Versions: 1.12.0 >Reporter: slim bouguerra >Assignee: Julian Hyde > > Druid does not support grouping by metrics, hence we can not push the group > by to druid. Instead we should generate a select query. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (CALCITE-1670) Count distinct on druid is translated to Cardinality aggregator which is approximate
[
https://issues.apache.org/jira/browse/CALCITE-1670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15895365#comment-15895365
]
Gian Merlino commented on CALCITE-1670:
---
Druid can do two aggregate passes using nested groupBys. I recently set up
Druid to use that Calcite rule you're talking about, and it works well. The
patch for that was: https://github.com/druid-io/druid/pull/3999. With that
applied, APPROX_COUNT_DISTINCT is always approximate, and COUNT(DISTINCT col)
is approximate by default but can be made exact through config. Not all queries
work in exact mode, for example you can't have two distinct counts on two
different columns (since it would generate a plan that Druid's runtime doesn't
support).
> Count distinct on druid is translated to Cardinality aggregator which is
> approximate
>
>
> Key: CALCITE-1670
> URL: https://issues.apache.org/jira/browse/CALCITE-1670
> Project: Calcite
> Issue Type: Bug
>Reporter: Nishant Bangarwa
>Assignee: Julian Hyde
>
> Right now count distinct on Druid is translated as a 'cardinality' aggregator
> which uses hyperloglog and return approximate results. See cardinality
> aggregator here - http://druid.io/docs/latest/querying/aggregations.html for
> details.
> https://github.com/apache/calcite/blob/master/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java#L721
> {code}
> case COUNT:
> if (aggCall.isDistinct()) {
> return new JsonCardinalityAggregation("cardinality", name, list);
> }
> return new JsonAggregation("count", name, only);
> {code}
> The current recommended way in druid to get exact counts is to do a nested
> groupby query.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (CALCITE-1601) DateRangeRules loses OR filters
[
https://issues.apache.org/jira/browse/CALCITE-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15883222#comment-15883222
]
Gian Merlino commented on CALCITE-1601:
---
I collected the failing tests in CALCITE-1658.
> DateRangeRules loses OR filters
> ---
>
> Key: CALCITE-1601
> URL: https://issues.apache.org/jira/browse/CALCITE-1601
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.11.0
>Reporter: Gian Merlino
>Assignee: Julian Hyde
> Fix For: 1.12.0
>
>
> I'm not sure why, but DateRangeRules can lose some filters. For example in
> this query:
> {code}
> SELECT COUNT(*) FROM druid.foo WHERE EXTRACT(YEAR FROM __time) = 2000 AND
> EXTRACT(MONTH FROM __time) IN (2, 3, 5)
> {code}
> The filter gets resolved to __time >= 2000-02-01 and __time < 2000-03-01,
> losing the months "3" and "5". Removing DateRangeRules.FILTER_INSTANCE from
> the planner's rule set stops this from happening.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Created] (CALCITE-1658) DateRangeRules issues
Gian Merlino created CALCITE-1658:
-
Summary: DateRangeRules issues
Key: CALCITE-1658
URL: https://issues.apache.org/jira/browse/CALCITE-1658
Project: Calcite
Issue Type: Bug
Components: core
Reporter: Gian Merlino
Assignee: Julian Hyde
Follow up to CALCITE-1601. In Druid's built in SQL module (not the Druid
adapter in Calcite), some unit tests fail when DateRangeRules.FILTER_INSTANCE
is enabled. These include the SQLs below. In all cases, the predicate was
incorrectly simplified to "false" and no Druid queries were made.
Removing DateRangeRules from the planner causes the results to be correct.
{code}
SELECT COUNT(*) FROM druid.foo
WHERE
(EXTRACT(YEAR FROM __time) = 2000 AND EXTRACT(MONTH FROM __time) IN (2,
3, 5))
OR (EXTRACT(YEAR FROM __time) = 2001 AND EXTRACT(MONTH FROM __time) = 1)
{code}
{code}
SELECT COUNT(*) FROM druid.foo
WHERE
EXTRACT(YEAR FROM __time) IN (2000, 2001) AND ( (EXTRACT(YEAR FROM
__time) = 2000 AND EXTRACT(MONTH FROM __time) IN (2, 3, 5))
OR (EXTRACT(YEAR FROM __time) = 2001 AND EXTRACT(MONTH FROM __time) = 1)
)
{code}
{code}
SELECT COUNT(*) FROM druid.foo
WHERE
EXTRACT(YEAR FROM __time) <> 2000 AND ( (EXTRACT(YEAR FROM __time) =
2000 AND EXTRACT(MONTH FROM __time) IN (2, 3, 5))
OR (EXTRACT(YEAR FROM __time) = 2001 AND EXTRACT(MONTH FROM __time) = 1)
)
{code}
{code}
SELECT COUNT(*) FROM druid.foo
WHERE
EXTRACT(MONTH FROM __time) IN (1, 2, 3, 5) AND ( (EXTRACT(YEAR FROM
__time) = 2000 AND EXTRACT(MONTH FROM __time) IN (2, 3, 5))
OR (EXTRACT(YEAR FROM __time) = 2001 AND EXTRACT(MONTH FROM __time) = 1)
)
{code}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
