Re: RelOptRulesTest.java and RelOptRulesTest.xml

2022-10-24 Thread Benchao Li
Thanks Alessandro for driving this. It's great to hear this is being improved. I suffered from this problem several times before. Alessandro Solimando 于2022年10月24日周一 17:25写道: > I have a working implementation in the PR 2948 > for CALCITE-5340 >

Re: RelOptRulesTest.java and RelOptRulesTest.xml

2022-10-24 Thread Alessandro Solimando
I have a working implementation in the PR 2948 for CALCITE-5340 , it's covering the following test suites: - HepPlannerTest - RelOptRulesTest - SqlHintsConverterTest - SqlToRelConverterTest -

Re: RelOptRulesTest.java and RelOptRulesTest.xml

2022-10-21 Thread Alessandro Solimando
Thank you guys for your input. Given that the official way to update the XML file (as per our doc) is to automatically generate it, I think it should fail if the actual and expected files differ, whatever the difference is. I will dig more into how DiffRepository works and make the test fail on

Re: RelOptRulesTest.java and RelOptRulesTest.xml

2022-10-21 Thread Stamatis Zampetakis
Thanks for taking care of this Alessandro! I believe the resources are still sorted as per [1] but it appears that there are still a few manual edits to the file. If we want to prevent such changes in the future it may be a good idea to add a post-class check In RelOptRulesTest (or somewhere in

Re: RelOptRulesTest.java and RelOptRulesTest.xml

2022-10-20 Thread Julian Hyde
Do you think that RelOptRulesTest should fail if the resources are not sorted alphabetically? (I thought it did that at some point… I may be wrong. Anyway, I am a fan of ‘fail fast’. The person who introduces the error should be the person to fix it.) > On Oct 20, 2022, at 9:20 AM, Alessandro

RelOptRulesTest.java and RelOptRulesTest.xml

2022-10-20 Thread Alessandro Solimando
Hello everyone, I have recently added tests to RelOptRulesTest.java, took the chance to update (maven instruction with gradle instructions, path has changed) and improved them a little bit. While testing the commands I have noticed that few PRs in the last months had tests added to