Fixed testAggregateExpr test

[yesh385]

This PR fixes a test called DruidMysqlRouteStrategyTest.testAggregateExpr which can be here.

1. What does this test do?

This test is responsible for testing the behavior of the route strategy when processing SQL queries with aggregate functions and different aliasing scenarios. It checks if the result set correctly identifies and names the aggregated columns.

2. Why does this test fail?

This test fails because we are checking if the HashMap returned by getMergeCols() method contains the column name as a key while not checking for case sensitivity.

3. How I fixed?

This PR fixes this issue by updating the key name into uppercase.

The assertion happens here:

Mycat-Server/src/test/java/io/mycat/route/DruidMysqlRouteStrategyTest.java

Line 960 in aacfc6b

Assert.assertTrue(rrs.getMergeCols().containsKey("COUNT2"));

You can run the following command to run the test:

mvn test -Dtest=io.mycat.route.DruidMysqlRouteStrategyTest#testAggregateExpr

Test Environment:

java version "1.8.0_202"
Apache Maven 3.6.3

[219sansim]

General questions

1. Can you check if changing the data structure from Map to TreeMap does not break other tests?
2. Can you explain why it is logical to modify code under test and not the actual test?
3. Can you explain why we're concerned only with case insensitive order and use the comparator String.CASE_INSENSITIVE_ORDER, what about other ascii orderings?

[219sansim]

You have changed the data structure for mergeCols to TreeMap, but going through the class I have found a method such as getMergeCols which has return type Map and not TreeMap. Intuitively, it seems to me that it should be changed to TreeMap as well for consistency sake, can you explain why you haven't changed?
Can you check through all the classes and ensure that all the data structures are consistent

[yesh385]

Thank you for the feedback. I was under the impression that we should not modify the actual test and only the code under the test. That is why I had modified the Map to a TreeMap. I have reverted the changes and simply updated the test code which also fixes the test.

[219sansim]

Can you write your PR description? How is this a flaky test and why does your fix work?

[yesh385]

I updated the PR description. This test is not flaky. It fails with mvn test itself. It passes with the NonDex tool after this fix.

[zzjas]

You can proceed to open a real PR. Once you open a real PR, please mark this tentative PR as Opened in your tentative_pr.csv file and also raise a PR to IDoFT marking this as Opened. Thanks!

[Rette66]

LGTM