Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-2484

Dynamic table tests give wrong results when running tests concurrently.

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.17.0
    • Fix Version/s: 1.18.0
    • Component/s: core
    • Labels:
      None

      Description

      What's happening

      When two dynamic table tests referencing to the same table name run concurrently, the results of the tests will be incorrect, causing the tests to fail.

      How to reproduce this bug

      As the condition to trigger this bug is strict (two dynamic table tests must reference the same table name, and they must run concurrently), it's hard to reproduce this bug in the current test set.

      I construct two mock test classes to reproduce this bug stably. The two test classes are the same except for their names. One of the test class is listed as follows:

      public class MockSqlValidatorTest1 extends SqlValidatorTestCase {
        // Member definition omitted.
      
        @Test
        public void testDynamicStar1() {
          final String sql = "select newid from (\n"
              + "  select *, NATION.N_NATION + 100 as newid\n"
              + "  from \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER)";
          sql(sql).type("RecordType(ANY NEWID) NOT NULL");
        }
      
        @Test
        public void testDynamicStar2() {
          final String sql = "select newid from (\n"
              + "  select *, NATION.N_NATION + 100 as newid\n"
              + "  from \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER)";
          sql(sql).type("RecordType(ANY NEWID) NOT NULL");
        }
      
        // 296 more test cases
      
        @Test
        public void testDynamicStar299() {
          final String sql = "select newid from (\n"
              + "  select *, NATION.N_NATION + 100 as newid\n"
              + "  from \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER)";
          sql(sql).type("RecordType(ANY NEWID) NOT NULL");
        }
      
        @Test
        public void testDynamicStar300() {
          final String sql = "select newid from (\n"
              + "  select *, NATION.N_NATION + 100 as newid\n"
              + "  from \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER)";
          sql(sql).type("RecordType(ANY NEWID) NOT NULL");
        }
      

      You can check these two test classes here and here so you can try them out.

      To reproduce this bug, run

      mvn -T 4 -Dtest=MockSqlValidatorTest* test -pl core
      

      to run these two test classes concurrently, the bug will occur.

      Why is this happening

      1. In the current implementation, when a test class wants to use a SqlTestFactory, it will use SqlTestFactory.INSTANCE, so every class using this factory actually shares the same factory instance.
      2. catalogReader is a member of SqlTestFactory, so every class actually shares the same catalogReader.
      3. As root schema is stored in catalog reader, table is stored in root schema, and row type is stored in table, every class actually has access to the same row type instance.
      4. As dynamic table will modify row type if a column name it wants to use doesn't exist, two test cases running concurrently and using the same table name may read and modify the same row type instance, causing the result of the test to be incorrect, thus causing the failure of the test.

      How to fix this bug

      What I've done in this commit is to remove SqlTestFactory.INSTANCE, and let every test class use a new instance of the factory, so that we can solve the concurrent modification problem.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                julianhyde Julian Hyde
                Reporter:
                TsReaper Caizhi Weng
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: