search

heavyai / heavyai-jdbc

A JDBC driver for connecting to an HeavyAI GPU database and running queries.
https://www.heavy.ai/
Other
9 stars 16 forks source link

Fix column ordering for batch inserts #1

Closed sripathikrishnan closed 5 years ago

sripathikrishnan commented 5 years ago

Originally, batch inserts ignore the columns specified in the insert clause. This was apparently fixed with this pull request - https://github.com/omnisci/omniscidb/pull/335. However, this pull request doesn't seem to work for all cases.

In this pull request, we wrote a test case that generates multiple permutations of columns in an insert statement. Without the fix, the test case fails. With the fix, the test case passes.

Without fix:

[ERROR] testOrderOfColumnsInInsertDoesntMatter(com.omnisci.jdbc.OmniSciBatchInsertOrderTest)  Time elapsed: 1.074 s  <<< FAILURE!
java.lang.AssertionError: select count(*) from student expected:<100> but was:<25>
    at com.omnisci.jdbc.OmniSciBatchInsertOrderTest.assertCount(OmniSciBatchInsertOrderTest.java:187)
    at com.omnisci.jdbc.OmniSciBatchInsertOrderTest.testOrderOfColumnsInInsertDoesntMatter(OmniSciBatchInsertOrderTest.java:110)```

After applying fix:

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.omnisci.jdbc.OmniSciGeomTest
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.184 s - in com.omnisci.jdbc.OmniSciGeomTest
[INFO] Running com.omnisci.jdbc.OmniSciPrepareTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.966 s - in com.omnisci.jdbc.OmniSciPrepareTest
[INFO] Running com.omnisci.jdbc.OmniSciDatabaseMetaDataTest
[INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 18.017 s - in com.omnisci.jdbc.OmniSciDatabaseMetaDataTest
[INFO] Running com.omnisci.jdbc.OmniSciBatchInsertOrderTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.373 s - in com.omnisci.jdbc.OmniSciBatchInsertOrderTest
[INFO] Running com.omnisci.jdbc.OmniSciConnectionTest
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.09 s - in com.omnisci.jdbc.OmniSciConnectionTest
[INFO] Running com.omnisci.jdbc.OmniSciColumnTypeTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.736 s - in com.omnisci.jdbc.OmniSciColumnTypeTest
[INFO] Running com.omnisci.jdbc.OmniSciStatementTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.66 s - in com.omnisci.jdbc.OmniSciStatementTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 22, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
Smyatkin-Maxim commented 5 years ago

Thanks @sripathikrishnan, will review it shortly

sripathikrishnan commented 5 years ago

@Smyatkin-Maxim - any idea when this will make it to maven central repository?

Smyatkin-Maxim commented 5 years ago

@sripathikrishnan I've just uploaded 4.7.1-SNAPSHOT into maven for you right now. The release is supposed to happen in a few days. To use the snapshot something like this will do:

  <repositories>
    <repository>
      <id>snapshots-repo</id>
      <url>https://oss.sonatype.org/content/repositories/snapshots</url>
      <releases><enabled>false</enabled></releases>
      <snapshots><enabled>true</enabled></snapshots>
    </repository>
  </repositories>
  <dependencies>
    <dependency>
      <groupId>com.omnisci</groupId>
      <artifactId>jdbc</artifactId>
      <version>4.7.1-SNAPSHOT</version>
    </dependency>
  </dependencies>
sripathikrishnan commented 5 years ago

@Smyatkin-Maxim - that worked really well, and saved a lot of effort for me. It is a pain maintaining a fork + separate artifacts - and I am glad I don't have to! Thanks a lot!

Smyatkin-Maxim commented 5 years ago

Sure, thanks for the fix