You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Jin Xing (Jira)" <ji...@apache.org> on 2019/11/06 10:04:00 UTC
[jira] [Updated] (CALCITE-3478) Reconstructure of materialized view
tests.
[ https://issues.apache.org/jira/browse/CALCITE-3478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jin Xing updated CALCITE-3478:
------------------------------
Description:
h2. *Motivation*
Currently there are two strategies for materialized view matching:
*strategy-1*. Substitution based (SubstitutionVisitor.java) [1]
*strategy-2*. Plan structural information based (AbstractMaterializedViewRule.java) [2]
The two strategies are controlled by a single connection config of "materializationsEnabled". Calcite will apply strategy-1 firstly and then strategy-2.
The two strategies are tested in a single integration test called MaterializationTest.java,
As a result we cannot run tests separately for a single strategy, which leads to:
1. When some new matching patterns are supported by strategy-1, we might need to update the old result plan, which was previously matched and generated by stragegy-2
e.g. [3], and corresponding testing pattern for stragegy-2 will be lost.
2. Some test failures are even hidden, e.g. MaterializationTest#testJoinMaterialization2 is not supported by stragegy-2, but strategy-1 lets the test passed.
3. Hard to test internals for SubstutionVisitor.java, e.g. [4] has to struggle and create a unit test
Of course we can add more system config or connection config just for testing and circle around some of the dilemas I mentioned above. But it will make the code messy. Materialized view matching str
ategies are so important and worth a through unit test and to be kept clean.
Additionally, this JIRA targets to clean the code of MaterializationTest.java. As more and more fixes get applied, this Java file tends to be messy:
1. Helping methods and testing methods are mixed without good order.
2. Lots of methods called checkMaterialize. We need to sort it out if there's need to add more params, e.g. [5]
3. Some tests are not concise enough, e.g. testJoinMaterialization9
h2. *Approach*
1. Create unit test MaterializedViewSubstitutionVisitorTest to test strategy-1
2. Create unit test MaterializedViewRelOptRulesTest to test strategy-2
3. Move tests from MaterializationTest to unit tests correspondingly, and keep MaterializationTest for integration tests.
[1] [https://calcite.apache.org/docs/materialized_views.html#substitution-via-rules-transformation]
[2] [https://calcite.apache.org/docs/materialized_views.html#rewriting-using-plan-structural-information]
[3] [https://github.com/apache/calcite/pull/1451/files#diff-d7e9e44fcb5fb1b98198415a3f78f167R1831]
[4] [https://github.com/apache/calcite/pull/1555]
[5] [https://github.com/apache/calcite/pull/1504]
was:
Reconstructure of materialized view tests.
Motivation of Reconstructure
Currently there are two strategies for materialized view matching:
strategy-1. Substitution based (SubstitutionVisitor.java) [1]
strategy-2. Plan structural information based (AbstractMaterializedViewRule.java) [2]
The two strategies are controlled by a single connection config of "materializationsEnabled". Calcite will apply strategy-1 firstly and then strategy-2.
The two strategies are tested in a single integration test called MaterializationTest.java,
As a result we cannot run tests separately for a single strategy, which leads to:
1. When some new matching patterns are supported by strategy-1, we might need to update the old result plan, which was previously matched and generated by stragegy-2
e.g. [3], and corresponding testing pattern for stragegy-2 will be lost.
2. Some test failures are even hidden, e.g. MaterializationTest#testJoinMaterialization2 is not supported by stragegy-2, but strategy-1 lets the test passed.
3. Hard to test internals for SubstutionVisitor.java, e.g. [4] has to struggle and create a unit test
Of course we can add more system config or connection config just for testing and circle around some of the dilemas I mentioned above. But it will make the code messy. Materialized view matching str
ategies are so important and worth a through unit test and to be kept clean.
Additionally, this JIRA targets to clean the code of MaterializationTest.java. As more and more fixes get applied, this Java file tends to be messy:
1. Helping methods and testing methods are mixed without good order.
2. Lots of methods called checkMaterialize. We need to sort it out if there's need to add more params, e.g. [5]
3. Some tests are not concise enough, e.g. testJoinMaterialization9
Approach of Reconstructure
1. Create unit test MaterializedViewSubstitutionVisitorTest to test strategy-1
2. Create unit test MaterializedViewRelOptRulesTest to test strategy-2
3. Move tests from MaterializationTest to unit tests correspondingly, and keep MaterializationTest for integration tests.
[1] https://calcite.apache.org/docs/materialized_views.html#substitution-via-rules-transformation
[2] https://calcite.apache.org/docs/materialized_views.html#rewriting-using-plan-structural-information
[3] https://github.com/apache/calcite/pull/1451/files#diff-d7e9e44fcb5fb1b98198415a3f78f167R1831
[4] https://github.com/apache/calcite/pull/1555
[5] https://github.com/apache/calcite/pull/1504
> Reconstructure of materialized view tests.
> ------------------------------------------
>
> Key: CALCITE-3478
> URL: https://issues.apache.org/jira/browse/CALCITE-3478
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Jin Xing
> Assignee: Jin Xing
> Priority: Major
>
> h2. *Motivation*
> Currently there are two strategies for materialized view matching:
> *strategy-1*. Substitution based (SubstitutionVisitor.java) [1]
> *strategy-2*. Plan structural information based (AbstractMaterializedViewRule.java) [2]
> The two strategies are controlled by a single connection config of "materializationsEnabled". Calcite will apply strategy-1 firstly and then strategy-2.
> The two strategies are tested in a single integration test called MaterializationTest.java,
> As a result we cannot run tests separately for a single strategy, which leads to:
> 1. When some new matching patterns are supported by strategy-1, we might need to update the old result plan, which was previously matched and generated by stragegy-2
> e.g. [3], and corresponding testing pattern for stragegy-2 will be lost.
> 2. Some test failures are even hidden, e.g. MaterializationTest#testJoinMaterialization2 is not supported by stragegy-2, but strategy-1 lets the test passed.
> 3. Hard to test internals for SubstutionVisitor.java, e.g. [4] has to struggle and create a unit test
> Of course we can add more system config or connection config just for testing and circle around some of the dilemas I mentioned above. But it will make the code messy. Materialized view matching str
> ategies are so important and worth a through unit test and to be kept clean.
> Additionally, this JIRA targets to clean the code of MaterializationTest.java. As more and more fixes get applied, this Java file tends to be messy:
> 1. Helping methods and testing methods are mixed without good order.
> 2. Lots of methods called checkMaterialize. We need to sort it out if there's need to add more params, e.g. [5]
> 3. Some tests are not concise enough, e.g. testJoinMaterialization9
> h2. *Approach*
> 1. Create unit test MaterializedViewSubstitutionVisitorTest to test strategy-1
> 2. Create unit test MaterializedViewRelOptRulesTest to test strategy-2
> 3. Move tests from MaterializationTest to unit tests correspondingly, and keep MaterializationTest for integration tests.
>
> [1] [https://calcite.apache.org/docs/materialized_views.html#substitution-via-rules-transformation]
> [2] [https://calcite.apache.org/docs/materialized_views.html#rewriting-using-plan-structural-information]
> [3] [https://github.com/apache/calcite/pull/1451/files#diff-d7e9e44fcb5fb1b98198415a3f78f167R1831]
> [4] [https://github.com/apache/calcite/pull/1555]
> [5] [https://github.com/apache/calcite/pull/1504]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)