Skip to content

[CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg) - #1852

Closed
vineetgarg02 wants to merge 1 commit into
apache:masterfrom
vineetgarg02:CALCITE-3848
Closed

[CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)#1852
vineetgarg02 wants to merge 1 commit into
apache:masterfrom
vineetgarg02:CALCITE-3848

Conversation

@vineetgarg02

Copy link
Copy Markdown
Contributor

This request fixes an issue in materialize view rewriting mapping logic

@jcamachor jcamachor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also run materialized views tests in Hive with this change? Due to complex plans and different interaction with other rules, those tests may catch issues that existing tests in Calcite do not.

if (rewritingMapping != null) {
pos = rewritingMapping.getTargetOpt(pos);
pos = rewritingMapping.get(pos).iterator().next();
if (pos == -1) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be -1 when we called getTargetOpt. However, can this be -1 now? Should we check for empty instead of == -1? Same for follow-up calls.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup good catch.

Comment thread core/src/test/java/org/apache/calcite/test/MaterializationTest.java Outdated
@jcamachor jcamachor added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Mar 11, 2020
@asfgit asfgit closed this in 18b9bc3 Mar 11, 2020
jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Mar 16, 2026
… on join keys fails with Mappings$NoElementException (Vineet Garg)

Close apache#1852
jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Mar 16, 2026
… on join keys fails with Mappings$NoElementException (Vineet Garg)

Close apache#1852
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants