Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.5.0
    • Component/s: avatica
    • Labels:
      None

      Description

      There is apparently a thread safety issue in RemoteDriverTest.testStatementLifecycle. Here is output from <a href="https://travis-ci.org/julianhyde/incubator-calcite/jobs/58869949">Travis</a>:

      ests run: 18, Failures: 1, Errors: 0, Skipped: 4, Time elapsed: 29.008 sec <<< FAILURE! - in org.apache.calcite.avatica.RemoteDriverTest
      testStatementLifecycle(org.apache.calcite.avatica.RemoteDriverTest)  Time elapsed: 1.807 sec  <<< FAILURE!
      java.lang.AssertionError: expected:<1> but was:<9>
      	at org.junit.Assert.fail(Assert.java:88)
      	at org.junit.Assert.failNotEquals(Assert.java:743)
      	at org.junit.Assert.assertEquals(Assert.java:118)
      	at org.junit.Assert.assertEquals(Assert.java:555)
      	at org.junit.Assert.assertEquals(Assert.java:542)
      	at org.apache.calcite.avatica.RemoteDriverTest.testStatementLifecycle(RemoteDriverTest.java:388)
      

      Presumably this occurs in Travis because several tests are running concurrently.

      I have disabled the test. We must fix this issue and re-enable the test.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          As part of the same task, remove the sleep in the next test:

            @Test public void testConnectionIsolation() throws Exception {
              // Wait 5s for all other tests to finish. (Sorry! Hack!)
              Thread.sleep(5000);
          

          One possible solution would be to add a connect string flag

          threaded=true

          that gives each thread its own connection, or Meta, or whatever.

          Show
          julianhyde Julian Hyde added a comment - As part of the same task, remove the sleep in the next test: @Test public void testConnectionIsolation() throws Exception { // Wait 5s for all other tests to finish. (Sorry! Hack!) Thread .sleep(5000); One possible solution would be to add a connect string flag threaded= true that gives each thread its own connection, or Meta, or whatever.
          Hide
          julianhyde Julian Hyde added a comment -

          Nick Dimiduk, can you please look into this? Not urgent - for 1.3 would be good enough.

          Show
          julianhyde Julian Hyde added a comment - Nick Dimiduk , can you please look into this? Not urgent - for 1.3 would be good enough.
          Hide
          julianhyde Julian Hyde added a comment -

          Disabled some tests in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/948e4567. Please re-enable when fixing this issue.

          Show
          julianhyde Julian Hyde added a comment - Disabled some tests in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/948e4567 . Please re-enable when fixing this issue.
          Hide
          julianhyde Julian Hyde added a comment - - edited

          Still getting this. If you run

          mvn clean && mvn install

          it fails about one time in 5. Another variation is:

          java.lang.AssertionError: null
          	at org.apache.calcite.avatica.Meta$MetaResultSet.count(Meta.java:388)
          	at org.apache.calcite.avatica.jdbc.JdbcMeta.prepareAndExecute(JdbcMeta.java:747)
          	at org.apache.calcite.avatica.remote.LocalService.apply(LocalService.java:140)
          	at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:264)
          	at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:245)
          	at org.apache.calcite.avatica.remote.LocalJsonService.apply(LocalJsonService.java:36)
          	at org.apache.calcite.avatica.RemoteDriverTest$LoggingLocalJsonService.apply(RemoteDriverTest.java:931)
          	at org.apache.calcite.avatica.remote.JsonService.apply(JsonService.java:217)
          	at org.apache.calcite.avatica.remote.RemoteMeta.prepareAndExecute(RemoteMeta.java:177)
          	at org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:477)
          	at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:109)
          	at org.apache.calcite.avatica.AvaticaStatement.execute(AvaticaStatement.java:121)
          	at org.apache.calcite.avatica.RemoteDriverTest.checkStatementExecute(RemoteDriverTest.java:364)
          	at org.apache.calcite.avatica.RemoteDriverTest.checkStatementExecute(RemoteDriverTest.java:344)
          	at org.apache.calcite.avatica.RemoteDriverTest.testStatementExecuteLocal(RemoteDriverTest.java:285)
          Show
          julianhyde Julian Hyde added a comment - - edited Still getting this. If you run mvn clean && mvn install it fails about one time in 5. Another variation is: java.lang.AssertionError: null at org.apache.calcite.avatica.Meta$MetaResultSet.count(Meta.java:388) at org.apache.calcite.avatica.jdbc.JdbcMeta.prepareAndExecute(JdbcMeta.java:747) at org.apache.calcite.avatica.remote.LocalService.apply(LocalService.java:140) at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:264) at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:245) at org.apache.calcite.avatica.remote.LocalJsonService.apply(LocalJsonService.java:36) at org.apache.calcite.avatica.RemoteDriverTest$LoggingLocalJsonService.apply(RemoteDriverTest.java:931) at org.apache.calcite.avatica.remote.JsonService.apply(JsonService.java:217) at org.apache.calcite.avatica.remote.RemoteMeta.prepareAndExecute(RemoteMeta.java:177) at org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:477) at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:109) at org.apache.calcite.avatica.AvaticaStatement.execute(AvaticaStatement.java:121) at org.apache.calcite.avatica.RemoteDriverTest.checkStatementExecute(RemoteDriverTest.java:364) at org.apache.calcite.avatica.RemoteDriverTest.checkStatementExecute(RemoteDriverTest.java:344) at org.apache.calcite.avatica.RemoteDriverTest.testStatementExecuteLocal(RemoteDriverTest.java:285)
          Hide
          elserj Josh Elser added a comment -

          Nick Dimiduk, can I swipe this from you? I have some cycles to dig into this.

          Show
          elserj Josh Elser added a comment - Nick Dimiduk , can I swipe this from you? I have some cycles to dig into this.
          Hide
          elserj Josh Elser added a comment -

          I am a little perplexed – I've been seeing an error, often like:

          java.sql.SQLException: error while executing SQL "create table TEST_TABLE(id int not null, msg varchar(3) not null)": java.sql.SQLSyntaxErrorException: object name already exists: TEST_TABLE in statement [create table TEST_TABLE(id int not null, msg varchar(3) not null)]
            at org.apache.calcite.avatica.Helper.createException(Helper.java:41)
            at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:112)
            at org.apache.calcite.avatica.AvaticaStatement.executeQuery(AvaticaStatement.java:130)
            at org.apache.calcite.avatica.RemoteDriverTest.testCreateInsertUpdateDrop(RemoteDriverTest.java:465)
            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
            at java.lang.reflect.Method.invoke(Method.java:606)
            at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
            at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
            at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
            at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
            at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
            at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
            at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
            at org.junit.rules.RunRules.evaluate(RunRules.java:20)
            at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
            at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
            at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
            at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
            at org.apache.maven.surefire.junitcore.pc.Scheduler$1.run(Scheduler.java:370)
            at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
            at java.util.concurrent.FutureTask.run(FutureTask.java:262)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
            at java.lang.Thread.run(Thread.java:745)
          Caused by: java.lang.RuntimeException: java.sql.SQLSyntaxErrorException: object name already exists: TEST_TABLE in statement [create table TEST_TABLE(id int not null, msg varchar(3) not null)]
            at org.apache.calcite.avatica.jdbc.JdbcMeta.propagate(JdbcMeta.java:704)
            at org.apache.calcite.avatica.jdbc.JdbcMeta.prepareAndExecute(JdbcMeta.java:762)
            at org.apache.calcite.avatica.remote.LocalService.apply(LocalService.java:141)
            at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:806)
            at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:1)
            at org.apache.calcite.avatica.remote.LocalJsonService.apply(LocalJsonService.java:36)
            at org.apache.calcite.avatica.RemoteDriverTest$LoggingLocalJsonService.apply(RemoteDriverTest.java:1074)
            at org.apache.calcite.avatica.remote.JsonService.apply(JsonService.java:121)
            at org.apache.calcite.avatica.remote.RemoteMeta.prepareAndExecute(RemoteMeta.java:175)
            at org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:477)
            at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:110)
            ... 24 more
          Caused by: java.sql.SQLSyntaxErrorException: object name already exists: TEST_TABLE in statement [create table TEST_TABLE(id int not null, msg varchar(3) not null)]
            at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source)
            at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source)
            at org.hsqldb.jdbc.JDBCStatement.fetchResult(Unknown Source)
            at org.hsqldb.jdbc.JDBCStatement.execute(Unknown Source)
            at org.apache.calcite.avatica.jdbc.JdbcMeta.prepareAndExecute(JdbcMeta.java:742)
            ... 33 more
          Caused by: org.hsqldb.HsqlException: object name already exists: TEST_TABLE
            at org.hsqldb.error.Error.error(Unknown Source)
            at org.hsqldb.error.Error.error(Unknown Source)
            at org.hsqldb.SchemaObjectSet.checkAdd(Unknown Source)
            at org.hsqldb.SchemaManager.checkSchemaObjectNotExists(Unknown Source)
            at org.hsqldb.StatementSchema.setOrCheckObjectName(Unknown Source)
            at org.hsqldb.StatementSchema.getResult(Unknown Source)
            at org.hsqldb.StatementSchema.execute(Unknown Source)
            at org.hsqldb.Session.executeCompiledStatement(Unknown Source)
            at org.hsqldb.Session.executeDirectStatement(Unknown Source)
            at org.hsqldb.Session.execute(Unknown Source)
            ... 36 more
          

          The curious thing to me is that it doesn't look like there were any overlapping executions by surefire (not a thread-safe problem), but it certainly acts like it only failing occasionally. I have to assume it's something in Avatica, but I'm confused how after apparently we drop the table and it still exists when we're essentially just wrapping the hsqldb driver. Will dig more, but hints are welcome.

          Show
          elserj Josh Elser added a comment - I am a little perplexed – I've been seeing an error, often like: java.sql.SQLException: error while executing SQL "create table TEST_TABLE(id int not null, msg varchar(3) not null)": java.sql.SQLSyntaxErrorException: object name already exists: TEST_TABLE in statement [create table TEST_TABLE(id int not null, msg varchar(3) not null)] at org.apache.calcite.avatica.Helper.createException(Helper.java:41) at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:112) at org.apache.calcite.avatica.AvaticaStatement.executeQuery(AvaticaStatement.java:130) at org.apache.calcite.avatica.RemoteDriverTest.testCreateInsertUpdateDrop(RemoteDriverTest.java:465) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27) at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55) at org.junit.rules.RunRules.evaluate(RunRules.java:20) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238) at org.apache.maven.surefire.junitcore.pc.Scheduler$1.run(Scheduler.java:370) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) at java.util.concurrent.FutureTask.run(FutureTask.java:262) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745) Caused by: java.lang.RuntimeException: java.sql.SQLSyntaxErrorException: object name already exists: TEST_TABLE in statement [create table TEST_TABLE(id int not null, msg varchar(3) not null)] at org.apache.calcite.avatica.jdbc.JdbcMeta.propagate(JdbcMeta.java:704) at org.apache.calcite.avatica.jdbc.JdbcMeta.prepareAndExecute(JdbcMeta.java:762) at org.apache.calcite.avatica.remote.LocalService.apply(LocalService.java:141) at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:806) at org.apache.calcite.avatica.remote.Service$PrepareAndExecuteRequest.accept(Service.java:1) at org.apache.calcite.avatica.remote.LocalJsonService.apply(LocalJsonService.java:36) at org.apache.calcite.avatica.RemoteDriverTest$LoggingLocalJsonService.apply(RemoteDriverTest.java:1074) at org.apache.calcite.avatica.remote.JsonService.apply(JsonService.java:121) at org.apache.calcite.avatica.remote.RemoteMeta.prepareAndExecute(RemoteMeta.java:175) at org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:477) at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:110) ... 24 more Caused by: java.sql.SQLSyntaxErrorException: object name already exists: TEST_TABLE in statement [create table TEST_TABLE(id int not null, msg varchar(3) not null)] at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source) at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source) at org.hsqldb.jdbc.JDBCStatement.fetchResult(Unknown Source) at org.hsqldb.jdbc.JDBCStatement.execute(Unknown Source) at org.apache.calcite.avatica.jdbc.JdbcMeta.prepareAndExecute(JdbcMeta.java:742) ... 33 more Caused by: org.hsqldb.HsqlException: object name already exists: TEST_TABLE at org.hsqldb.error.Error.error(Unknown Source) at org.hsqldb.error.Error.error(Unknown Source) at org.hsqldb.SchemaObjectSet.checkAdd(Unknown Source) at org.hsqldb.SchemaManager.checkSchemaObjectNotExists(Unknown Source) at org.hsqldb.StatementSchema.setOrCheckObjectName(Unknown Source) at org.hsqldb.StatementSchema.getResult(Unknown Source) at org.hsqldb.StatementSchema.execute(Unknown Source) at org.hsqldb.Session.executeCompiledStatement(Unknown Source) at org.hsqldb.Session.executeDirectStatement(Unknown Source) at org.hsqldb.Session.execute(Unknown Source) ... 36 more The curious thing to me is that it doesn't look like there were any overlapping executions by surefire (not a thread-safe problem), but it certainly acts like it only failing occasionally. I have to assume it's something in Avatica, but I'm confused how after apparently we drop the table and it still exists when we're essentially just wrapping the hsqldb driver. Will dig more, but hints are welcome.
          Hide
          julianhyde Julian Hyde added a comment -

          I think it's because hsqldb is embedded and shared among all threads.

          Show
          julianhyde Julian Hyde added a comment - I think it's because hsqldb is embedded and shared among all threads.
          Hide
          elserj Josh Elser added a comment -

          I think it's because hsqldb is embedded and shared among all threads.

          That was my initial hunch, but then I was confused to see no other tests using that same table and (perhaps incorrectly) assumed that hsqldb would be threadsafe.

          I'll have to dig through their docs. I haven't used it myself, but got some general familiarity today figuring how the Scott stuff worked.

          Thanks for the hint. Will pursue.

          Show
          elserj Josh Elser added a comment - I think it's because hsqldb is embedded and shared among all threads. That was my initial hunch, but then I was confused to see no other tests using that same table and (perhaps incorrectly) assumed that hsqldb would be threadsafe. I'll have to dig through their docs. I haven't used it myself, but got some general familiarity today figuring how the Scott stuff worked. Thanks for the hint. Will pursue.
          Hide
          elserj Josh Elser added a comment -

          Well, I've been playing around with the hsqldb properties in the JDBC URI to no success. I haven't found any documentation that says concurrent access to a single in-memory instance won't work correctly (on the contrary, I've read that this should be just fine).

          I do want to ask before going farther down the rabbit hole: I assume removing the concurrent test-runner config from surefire is undesirable?

          I haven't seen a way to create new instances of the in-memory instance on a per-thread basis as the actual instance is hidden behind the driver. Will have to investigate with fresh eyes though before throwing the towel.

          Show
          elserj Josh Elser added a comment - Well, I've been playing around with the hsqldb properties in the JDBC URI to no success. I haven't found any documentation that says concurrent access to a single in-memory instance won't work correctly (on the contrary, I've read that this should be just fine). I do want to ask before going farther down the rabbit hole: I assume removing the concurrent test-runner config from surefire is undesirable? I haven't seen a way to create new instances of the in-memory instance on a per-thread basis as the actual instance is hidden behind the driver. Will have to investigate with fresh eyes though before throwing the towel.
          Hide
          julianhyde Julian Hyde added a comment -

          Since we're interested in general correctness rather than concurrency, I think it would be OK to make this test single-threaded. To be clear: at most one test method in this class would be active at a time; but other tests might be running.

          Show
          julianhyde Julian Hyde added a comment - Since we're interested in general correctness rather than concurrency, I think it would be OK to make this test single-threaded. To be clear: at most one test method in this class would be active at a time; but other tests might be running.
          Hide
          elserj Josh Elser added a comment -

          To be clear: at most one test method in this class would be active at a time; but other tests might be running.

          It doesn't look like this is actually sufficient. Changing <parallel>both</parallel> to <parallel>classes</parallel> still exhibits the same spurious failures. I'm guessing that's due to RemoteMetaTest also using the same hsqldb under the hood. No parallel does seem to be passing for me (letting it run in a loop) with one of the previously ignored tests un-ignored.

          Show
          elserj Josh Elser added a comment - To be clear: at most one test method in this class would be active at a time; but other tests might be running. It doesn't look like this is actually sufficient. Changing <parallel>both</parallel> to <parallel>classes</parallel> still exhibits the same spurious failures. I'm guessing that's due to RemoteMetaTest also using the same hsqldb under the hood. No parallel does seem to be passing for me (letting it run in a loop) with one of the previously ignored tests un-ignored.
          Hide
          julianhyde Julian Hyde added a comment -

          How about using a java.util.concurrent.locks.Lock to manage access to the hsqldb? Tests that require it could use try ... finally. Later we might be able to downgrade some tests to just a read lock, so they can run in parallel with other read tests.

          Show
          julianhyde Julian Hyde added a comment - How about using a java.util.concurrent.locks.Lock to manage access to the hsqldb? Tests that require it could use try ... finally. Later we might be able to downgrade some tests to just a read lock, so they can run in parallel with other read tests.
          Hide
          elserj Josh Elser added a comment -

          How about using a java.util.concurrent.locks.Lock to manage access to the hsqldb

          Sure, that's a reasonable approach. I hadn't considered something so coarse. Should work though. I'll try that out. Thanks for the suggestion.

          Show
          elserj Josh Elser added a comment - How about using a java.util.concurrent.locks.Lock to manage access to the hsqldb Sure, that's a reasonable approach. I hadn't considered something so coarse. Should work though. I'll try that out. Thanks for the suggestion.
          Hide
          elserj Josh Elser added a comment -

          Git-formatted patch (2 commits inside – use `git am`). The commits..

          1. Revert Julian's old test-ignore additions
          2. Add a coarse lock across tests that use the ScottDB/HSQLDB to fix the concurrency issues and get the tests passing again.

          Threw the test in a while loop locally with success.

          Show
          elserj Josh Elser added a comment - Git-formatted patch (2 commits inside – use `git am`). The commits.. Revert Julian's old test-ignore additions Add a coarse lock across tests that use the ScottDB/HSQLDB to fix the concurrency issues and get the tests passing again. Threw the test in a while loop locally with success.
          Hide
          julianhyde Julian Hyde added a comment -

          Travis failed (on 1.7 and 1.8) with

           RemoteDriverTest.testConnectionIsolation:688 connection cache should start empty expected:<0> but was:<1>

          ; see https://travis-ci.org/julianhyde/incubator-calcite/builds/79737579. I don't think this is related to hsqldb synchronization. I will try disabling just that test.

          Show
          julianhyde Julian Hyde added a comment - Travis failed (on 1.7 and 1.8) with RemoteDriverTest.testConnectionIsolation:688 connection cache should start empty expected:<0> but was:<1> ; see https://travis-ci.org/julianhyde/incubator-calcite/builds/79737579 . I don't think this is related to hsqldb synchronization. I will try disabling just that test.
          Hide
          elserj Josh Elser added a comment -

          Ah, yeah, I saw a similar failure on the statement cache. Hadn't seen this one. Thanks, will post second patch soon.

          Show
          elserj Josh Elser added a comment - Ah, yeah, I saw a similar failure on the statement cache. Hadn't seen this one. Thanks, will post second patch soon.
          Hide
          julianhyde Julian Hyde added a comment -

          By the way, you can also make a pull request. That's easier for me because I can see what you are changing between successive versions of the patch. It's easy for one of use to rebase and squash when the change is ready.

          Show
          julianhyde Julian Hyde added a comment - By the way, you can also make a pull request. That's easier for me because I can see what you are changing between successive versions of the patch. It's easy for one of use to rebase and squash when the change is ready.
          Hide
          elserj Josh Elser added a comment -

          Sounds good. Happy to make a PR for things instead . Patches are just ingrained in my workflow by default now.

          Show
          elserj Josh Elser added a comment - Sounds good. Happy to make a PR for things instead . Patches are just ingrained in my workflow by default now.
          Hide
          julianhyde Julian Hyde added a comment -

          I've run several test iterations of https://github.com/julianhyde/incubator-calcite/commit/ad7fbb8f3be28f145eb0fa68f3280cd307cef8fd and things look good. We just need a proper fix for testConnectionIsolation and I have an idea – can I take over?

          Show
          julianhyde Julian Hyde added a comment - I've run several test iterations of https://github.com/julianhyde/incubator-calcite/commit/ad7fbb8f3be28f145eb0fa68f3280cd307cef8fd and things look good. We just need a proper fix for testConnectionIsolation and I have an idea – can I take over?
          Hide
          elserj Josh Elser added a comment -

          We just need a proper fix for testConnectionIsolation and I have an idea – can I take over?

          Sorry, I've gotten pulled a couple of different directions and didn't get back to this right away. I have an updated commit locally which just clears the connection cache here and that's been working for me.

          How about I send up a PR and if you have something better, please by all means put it in?

          Show
          elserj Josh Elser added a comment - We just need a proper fix for testConnectionIsolation and I have an idea – can I take over? Sorry, I've gotten pulled a couple of different directions and didn't get back to this right away. I have an updated commit locally which just clears the connection cache here and that's been working for me. How about I send up a PR and if you have something better, please by all means put it in?
          Hide
          elserj Josh Elser added a comment -

          Opened the PR with the changes that were working for me.

          Show
          elserj Josh Elser added a comment - Opened the PR with the changes that were working for me.
          Hide
          julianhyde Julian Hyde added a comment -
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/8f2fa3a5 . Thanks for the patch, Josh Elser !
          Hide
          elserj Josh Elser added a comment -

          Thanks, Julian!

          Show
          elserj Josh Elser added a comment - Thanks, Julian!
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Resolved in release 1.5.0 (2015-11-10)

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.5.0 (2015-11-10)

            People

            • Assignee:
              elserj Josh Elser
              Reporter:
              julianhyde Julian Hyde
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development