[jira] [Commented] (CALCITE-3478) Reconstructure of materialized view tests.

["Julian Hyde (Jira)" <ji...@apache.org>]

    [ 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)