+1，very good proposal, really cool. Thank you for doing these. After merging your first PR, I will see what I can do.
Maybe we can open a jira case and split the subtasks to claim it. Best wishes, Cancai Cai On 2025/02/14 20:18:45 Julian Hyde wrote: > 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 >
