[
https://issues.apache.org/jira/browse/CALCITE-3478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16988178#comment-16988178
]
Julian Hyde commented on CALCITE-3478:
--------------------------------------
Can you change Jira summary and commit message to "Restructure tests for
materialized views". "Reconstructure" is not a word.
> 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
> Labels: pull-request-available
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> 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:
> # 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.
> # Some test failures are even hidden, e.g.
> MaterializationTest#testJoinMaterialization2 should but failed to be
> supported by stragegy-2. However strategy-1 lets the test passed.
> # 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 dilemmas I mentioned above. But it will make
> the code messy. Materialized view matching strategies 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:
> # Helping methods and testing methods are mixed without good order.
> # Lots of methods called checkMaterialize. We need to sort it out if there's
> need to add more params, e.g. [5]
> # 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)
