[jira] [Commented] (CALCITE-2484) Dynamic table tests give wrong results when running tests concurrently.
[
https://issues.apache.org/jira/browse/CALCITE-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16604839#comment-16604839
]
Vladimir Sitnikov commented on CALCITE-2484:
Fixed in
https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=23eb11eb3b59e496cdc865926537a76fff53c165
> Dynamic table tests give wrong results when running tests concurrently.
> ---
>
> Key: CALCITE-2484
> URL: https://issues.apache.org/jira/browse/CALCITE-2484
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0
>Reporter: Caizhi Weng
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.18.0
>
>
> h2. 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.
> h2. 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:
> {code:java}
> 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");
> }
> {code}
> You can check these two test classes
> [here|https://paste.ubuntu.com/p/rcWYjMzMjf/] and
> [here|https://paste.ubuntu.com/p/Tb2VTz74Xv/] so you can try them out.
> To reproduce this bug, run
> {code}
> mvn -T 4 -Dtest=MockSqlValidatorTest* test -pl core
> {code}
> to run these two test classes concurrently, the bug will occur.
> h2. Why is this happening
> # 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.
> # {{catalogReader}} is a member of {{SqlTestFactory}}, so every class
> actually shares the same {{catalogReader}}.
> # 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.
> # 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.
> h2. 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.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2484) Dynamic table tests give wrong results when running tests concurrently.
[
https://issues.apache.org/jira/browse/CALCITE-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16596719#comment-16596719
]
Julian Hyde commented on CALCITE-2484:
--
DynamicSchema is not thread-safe, and is not intended to be.
I don't think we should make NameSet thread-safe. But if we can make it give
better errors when it is used unsafely, that would be great.
> Dynamic table tests give wrong results when running tests concurrently.
> ---
>
> Key: CALCITE-2484
> URL: https://issues.apache.org/jira/browse/CALCITE-2484
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0
>Reporter: Caizhi Weng
>Assignee: Julian Hyde
>Priority: Major
>
> h2. 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.
> h2. 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:
> {code:java}
> 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");
> }
> {code}
> You can check these two test classes
> [here|https://paste.ubuntu.com/p/rcWYjMzMjf/] and
> [here|https://paste.ubuntu.com/p/Tb2VTz74Xv/] so you can try them out.
> To reproduce this bug, run
> {code}
> mvn -T 4 -Dtest=MockSqlValidatorTest* test -pl core
> {code}
> to run these two test classes concurrently, the bug will occur.
> h2. Why is this happening
> # 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.
> # {{catalogReader}} is a member of {{SqlTestFactory}}, so every class
> actually shares the same {{catalogReader}}.
> # 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.
> # 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.
> h2. 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.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2484) Dynamic table tests give wrong results when running tests concurrently.
[
https://issues.apache.org/jira/browse/CALCITE-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16596116#comment-16596116
]
Vladimir Sitnikov commented on CALCITE-2484:
This seems to be relevant as well (macOs +
https://github.com/apache/calcite/pull/807):
{noformat}java.lang.IllegalArgumentException
at
com.google.common.base.Preconditions.checkArgument(Preconditions.java:108)
at
com.google.common.collect.ImmutableSortedSet.subSet(ImmutableSortedSet.java:607)
at
com.google.common.collect.ImmutableSortedSet.subSet(ImmutableSortedSet.java:57)
at org.apache.calcite.util.NameHelper.lambda$set$2(NameHelper.java:105)
at
org.apache.calcite.util.NameHelper.applyFloorCeiling(NameHelper.java:84)
at org.apache.calcite.util.NameHelper.set(NameHelper.java:102)
at org.apache.calcite.util.NameSet.range(NameSet.java:93)
at
org.apache.calcite.jdbc.CachingCalciteSchema.getImplicitType(CachingCalciteSchema.java:149)
at org.apache.calcite.jdbc.CalciteSchema.getType(CalciteSchema.java:371)
at
org.apache.calcite.sql.validate.SqlValidatorUtil.getTypeEntry(SqlValidatorUtil.java:975)
at
org.apache.calcite.prepare.CalciteCatalogReader.getNamedType(CalciteCatalogReader.java:172)
at
org.apache.calcite.test.catalog.MockCatalogReaderSimple.getNamedType(MockCatalogReaderSimple.java:65)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.getValidatedNodeTypeIfKnown(SqlValidatorImpl.java:1571)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.expandStar(SqlValidatorImpl.java:347)
at
org.apache.calcite.sql.validate.SqlValidatorImpl$OrderExpressionExpander.nthSelectItem(SqlValidatorImpl.java:5730)
at
org.apache.calcite.sql.validate.SqlValidatorImpl$OrderExpressionExpander.visit(SqlValidatorImpl.java:5756)
at
org.apache.calcite.sql.validate.SqlValidatorImpl$OrderExpressionExpander.visit(SqlValidatorImpl.java:5679)
at org.apache.calcite.sql.SqlIdentifier.accept(SqlIdentifier.java:334)
at
org.apache.calcite.sql.validate.SqlValidatorImpl$OrderExpressionExpander.go(SqlValidatorImpl.java:5692)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.expandOrderExpr(SqlValidatorImpl.java:3856)
at
org.apache.calcite.sql.validate.OrderByScope.validateExpr(OrderByScope.java:122)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateExpr(SqlValidatorImpl.java:4094)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateOrderItem(SqlValidatorImpl.java:3851)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateOrderList(SqlValidatorImpl.java:3819)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelect(SqlValidatorImpl.java:3305)
at
org.apache.calcite.sql.validate.SelectNamespace.validateImpl(SelectNamespace.java:60)
at
org.apache.calcite.sql.validate.AbstractNamespace.validate(AbstractNamespace.java:84)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateNamespace(SqlValidatorImpl.java:969)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateQuery(SqlValidatorImpl.java:945)
at org.apache.calcite.sql.SqlSelect.validate(SqlSelect.java:225)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validateScopedExpression(SqlValidatorImpl.java:920)
at
org.apache.calcite.sql.validate.SqlValidatorImpl.validate(SqlValidatorImpl.java:628)
at
org.apache.calcite.sql.test.SqlTesterImpl.assertExceptionIsThrown(SqlTesterImpl.java:130)
at
org.apache.calcite.test.SqlValidatorTestCase$Sql.ok(SqlValidatorTestCase.java:591)
at
org.apache.calcite.test.SqlValidatorTest.testOrderByColumn(SqlValidatorTest.java:4574)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at
org.apache.maven.surefire.junitcore.pc.Scheduler$1.run(Scheduler.java:410)
at
[jira] [Commented] (CALCITE-2484) Dynamic table tests give wrong results when running tests concurrently.
[
https://issues.apache.org/jira/browse/CALCITE-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16591433#comment-16591433
]
Vladimir Sitnikov commented on CALCITE-2484:
I'm inclined to use the other way around: make dynamic tables an opt-in, so
regular catalog could be reused.
I suggest
1) Move "dynamic" tests from {{SqlValidatorTest}} to a new class
{{SqlValidatorDynamicTest}} (a mere split of a large class)
2) Move "dynamic" schema from {{MockCatalogReaderSimple}} to
{{MockCatalogReaderDynamic}}
3) Move "org.apache.calcite.test.SqlToRelTestBase#getTesterWithDynamicTable" to
{{MockCatalogReaderDynamic}} as well
2) Add {{withDynamicCatalog}} to {{org.apache.calcite.sql.test.SqlTester}}
3) Override {{SqlValidatorDynamicTest#getTester}} as {{...
withDynamicCatalog()}}
> Dynamic table tests give wrong results when running tests concurrently.
> ---
>
> Key: CALCITE-2484
> URL: https://issues.apache.org/jira/browse/CALCITE-2484
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0
>Reporter: Caizhi Weng
>Assignee: Julian Hyde
>Priority: Major
>
> h2. 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.
> h2. 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:
> {code:java}
> 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");
> }
> {code}
> You can check these two test classes
> [here|https://paste.ubuntu.com/p/rcWYjMzMjf/] and
> [here|https://paste.ubuntu.com/p/Tb2VTz74Xv/] so you can try them out.
> To reproduce this bug, run
> {code}
> mvn -T 4 -Dtest=MockSqlValidatorTest* test -pl core
> {code}
> to run these two test classes concurrently, the bug will occur.
> h2. Why is this happening
> # 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.
> # {{catalogReader}} is a member of {{SqlTestFactory}}, so every class
> actually shares the same {{catalogReader}}.
> # 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.
> # 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.
> h2. 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.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2484) Dynamic table tests give wrong results when running tests concurrently.
[
https://issues.apache.org/jira/browse/CALCITE-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589978#comment-16589978
]
Caizhi Weng commented on CALCITE-2484:
--
[~vladimirsitnikov] This issue will only present when the cache still exists.
As cache is removed in CALCITE 2435, this problem doesn't exist any more. Sorry
I didn't notice that commit.
> Dynamic table tests give wrong results when running tests concurrently.
> ---
>
> Key: CALCITE-2484
> URL: https://issues.apache.org/jira/browse/CALCITE-2484
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0
>Reporter: Caizhi Weng
>Assignee: Julian Hyde
>Priority: Major
>
> When running tests on the cluster of my company, I always experience the
> failure of the test {{SqlValidatorTest::testDynamicStar2()}}. After
> investigation, I discover that it is triggered by the cache in
> {{DefaultSqlTestFactory}} introduced in [this
> commit|https://github.com/apache/calcite/commit/39c22f0c8b7b5b46a152f432e8708ce73ace1ef7].
> The failure of the test case is because:
> # In the current implementation, when a test class wants to use a
> {{DefaultSqlTestFactory}}, it will use {{DefaultSqlTestFactory.INSTANCE}}, so
> every class using this factory actually shares the same factory instance.
> # {{cache}} is a private member of {{DefaultSqlTestFactory}}, so every class
> actually shares the same {{cache}}.
> # The key of {{cache}} is of {{SqlTestFactory}} type. As every class shares
> the same factory instance, every class actually shares the same cache key.
> # As catalog reader is stored in cache, 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.
> # 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 the failure of the test.
> What we need to do is to remove {{DefaultSqlTestFactory.INSTANCE}}, and let
> every test class use a new instance of the factory, so that we can solve the
> concurrent modification problem.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2484) Dynamic table tests give wrong results when running tests concurrently.
[
https://issues.apache.org/jira/browse/CALCITE-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589959#comment-16589959
]
Vladimir Sitnikov commented on CALCITE-2484:
[~TsReaper], DefaultSqlTestFactory does not exists anymore. Could you please
re-check if the issue is still present?
> Dynamic table tests give wrong results when running tests concurrently.
> ---
>
> Key: CALCITE-2484
> URL: https://issues.apache.org/jira/browse/CALCITE-2484
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Caizhi Weng
>Assignee: Julian Hyde
>Priority: Major
>
> When running tests on the cluster of my company, I always experience the
> failure of the test {{SqlValidatorTest::testDynamicStar2()}}. After
> investigation, I discover that it is triggered by the cache in
> {{DefaultSqlTestFactory}} introduced in [this
> commit|https://github.com/apache/calcite/commit/39c22f0c8b7b5b46a152f432e8708ce73ace1ef7].
> The failure of the test case is because:
> # In the current implementation, when a test class wants to use a
> {{DefaultSqlTestFactory}}, it will use {{DefaultSqlTestFactory.INSTANCE}}, so
> every class using this factory actually shares the same factory instance.
> # {{cache}} is a private member of {{DefaultSqlTestFactory}}, so every class
> actually shares the same {{cache}}.
> # The key of {{cache}} is of {{SqlTestFactory}} type. As every class shares
> the same factory instance, every class actually shares the same cache key.
> # As catalog reader is stored in cache, 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.
> # 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 the failure of the test.
> What we need to do is to remove {{DefaultSqlTestFactory.INSTANCE}}, and let
> every test class use a new instance of the factory, so that we can solve the
> concurrent modification problem.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
