Commit 88f125541, "Reduce HepPlannerTest#testRuleApplyCount complexity"

2018-09-09 Thread Julian Hyde
Vladimir, I strongly disagree with your commit 88f125541. You have removed test code that is useful in creating a high-quality product. I presume that your goal is to make the test suite run a little faster. I think you should back out your commit. I sent a previous email on the subject but you

Re: Commit 88f125541, "Reduce HepPlannerTest#testRuleApplyCount complexity"

2018-09-09 Thread Vladimir Sitnikov
Julian>I strongly disagree with your commit 88f125541. You have removed test code that is useful in creating a high-quality product. Could you please provide technical justification? Note: the test is still there. Note2: the test still runs in both Travis and Apache Jenkins. The test still runs f

Re: Commit 88f125541, "Reduce HepPlannerTest#testRuleApplyCount complexity"

2018-09-09 Thread Julian Hyde
You seem to believe that the only purpose of a test is to test the specific bug or change for which it was introduced. That is indeed the philosophy of test-driven development, but there are more things in heaven and earth than TDD. Calcite is a complex project, and needs complex tests to shake

Re: Commit 88f125541, "Reduce HepPlannerTest#testRuleApplyCount complexity"

2018-09-09 Thread Vladimir Sitnikov
Julian>You seem to believe that the only purpose of a test is to test the specific bug or change for which it was introduced I'm afraid you seem to put words in my mouth. Note: so far I have technical reasons to keep the test with reduced number of lines while you don't. Note2: original test inc

Re: Commit 88f125541, "Reduce HepPlannerTest#testRuleApplyCount complexity"

2018-09-09 Thread Julian Hyde
The technical justification is that the code — yes, sql is code — that you removed might have found a bug someday. A deep relational algebra tree places stresses on hep planner (and other parts of the system) that we cannot predict. The (implicit) expected behavior is that it doesn’t crash, prod