+1 for this great proposal! Maybe we can file a comprehensive JIRA case and slice it up after your first merging, for those who are interested in refactoring the tests and what’s done or not.
Best ZheHu ---- Replied Message ---- | From | Xiong Duan<xi...@apache.org> | | Date | 02/15/2025 16:25 | | To | <dev@calcite.apache.org> | | Subject | Re: Fragmentation of dialect tests | Julian, Thank you for doing these. After merging your first PR, I will see what I can do. Julian Hyde <jh...@apache.org> 于2025年2月15日周六 04:20写道： Calcite committers, We have too many dialect tests (RelToSqlConverterTest alone is 10,000 lines long) and we have too few (for many functions and operators, we test the translation to SQL in only one or two dialects, and we never actually execute that SQL). I have a suggestion that I think will make a big difference: reduce the fragmentation in RelToSqlConverterTest [1] by making one method translate to several dialects. For example, testSubstring [2] is a good example (it tests the SUBSTRING function in 11 dialects) but it is accompanied by bad examples (testSubstringInSpark [3] and testHiveSubstring [4] duplicate the functionality of testSubstring for one dialect each). This is an example of "tragedy of the commons". Many contributors have an incentive to improve translation for just one dialect, but less incentive to make the dialect system better for everyone. The solution is for committers to force alignment - by requiring that contributors refactor existing tests rather than always adding new ones. I know it's no fun to be the 'bad cop', but cops tend to be necessary to allow the commons to prosper. I am working on radically improving the dialect tests [4][5] - so that each test generates SQL for, and optionally executes against, the dozen or so 'core dialects' - and rationalizing the existing tests will complement that change. I would love people's help refactoring the tests after my first commit is merged, but please don't make any major changes to RelToSqlConverterTest before that merge has happened. Julian [1] https://github.com/apache/calcite/blob/main/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java [2] https://github.com/apache/calcite/blob/2ddfb0eb00bd87b040e4ca743a7451034dd28f98/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L5451 [3] https://github.com/apache/calcite/blob/2ddfb0eb00bd87b040e4ca743a7451034dd28f98/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L7567 [3] https://github.com/apache/calcite/blob/2ddfb0eb00bd87b040e4ca743a7451034dd28f98/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L3305 [4] https://github.com/julianhyde/calcite/commits/5529-dialect-tests/ [5] https://issues.apache.org/jira/browse/CALCITE-5529
