You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tokoko (via GitHub)" <gi...@apache.org> on 2023/02/23 21:01:03 UTC

[GitHub] [arrow-adbc] tokoko opened a new pull request, #474: [JAVA] Fix/Refactor JDBC constraints

tokoko opened a new pull request, #474:
URL: https://github.com/apache/arrow-adbc/pull/474

   Fixes https://github.com/apache/arrow-adbc/issues/471. 
   
   - ObjectMetadataBuilder has been partly rewritten to use Writers instead of directly writing to Vectors, allowing to get rid of much of row index accounting.
   - ObjectMetadataBuilder handles UNIQUE constraints.
   - Added tests for constraints. Right now SQL statements are issued to create PK and FK keys directly without any changes in Quirks. Seems to be sufficient for both Derby and Postgres. Probably will have to be changed for more complicated scenarios.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] tokoko commented on a diff in pull request #474: feat(java/driver/jdbc): expose constraints in GetObjects

Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #474:
URL: https://github.com/apache/arrow-adbc/pull/474#discussion_r1117741897


##########
java/driver/jdbc/src/main/java/org/apache/arrow/adbc/driver/jdbc/ObjectMetadataBuilder.java:
##########
@@ -21,11 +21,10 @@
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;

Review Comment:
   agreed, not sure how I missed this



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #474: [Java] Fix/Refactor JDBC constraints

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #474:
URL: https://github.com/apache/arrow-adbc/pull/474#discussion_r1117261052


##########
java/driver/validation/src/main/java/org/apache/arrow/adbc/driver/testsuite/SqlTestUtil.java:
##########
@@ -69,4 +69,140 @@ public Schema ingestTableIntsStrs(
     }
     return schema;
   }
+
+  /** Load a table with composite primary key */
+  public Schema ingestTableWithConstraints(
+      BufferAllocator allocator, AdbcConnection connection, String tableName) throws Exception {
+    tableName = quirks.caseFoldTableName(tableName);
+    final Schema schema =
+        new Schema(
+            Arrays.asList(
+                Field.notNullable(
+                    quirks.caseFoldColumnName("INTS"), new ArrowType.Int(32, /*signed=*/ true)),
+                Field.nullable(quirks.caseFoldColumnName("INTS2"), new ArrowType.Int(32, true))));
+    try (final VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator)) {
+      final IntVector ints = (IntVector) root.getVector(0);
+      final IntVector strs = (IntVector) root.getVector(1);
+
+      ints.allocateNew(4);
+      ints.setSafe(0, 0);
+      ints.setSafe(1, 1);
+      ints.setSafe(2, 2);
+      ints.setSafe(3, 3);
+      strs.allocateNew(4);
+      strs.setSafe(0, 10);
+      strs.setSafe(1, 11);
+      strs.setSafe(2, 12);
+      strs.setSafe(3, 13);
+      root.setRowCount(4);
+      try (final AdbcStatement stmt = connection.bulkIngest(tableName, BulkIngestMode.CREATE)) {
+        stmt.bind(root);
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery("ALTER TABLE " + tableName + " ALTER COLUMN INTS SET NOT NULL");

Review Comment:
   Maybe make it a default method of SqlValidationQuirks to generate this statement given a table and column name



##########
java/driver/validation/src/main/java/org/apache/arrow/adbc/driver/testsuite/SqlTestUtil.java:
##########
@@ -69,4 +69,140 @@ public Schema ingestTableIntsStrs(
     }
     return schema;
   }
+
+  /** Load a table with composite primary key */
+  public Schema ingestTableWithConstraints(
+      BufferAllocator allocator, AdbcConnection connection, String tableName) throws Exception {
+    tableName = quirks.caseFoldTableName(tableName);
+    final Schema schema =
+        new Schema(
+            Arrays.asList(
+                Field.notNullable(
+                    quirks.caseFoldColumnName("INTS"), new ArrowType.Int(32, /*signed=*/ true)),
+                Field.nullable(quirks.caseFoldColumnName("INTS2"), new ArrowType.Int(32, true))));
+    try (final VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator)) {
+      final IntVector ints = (IntVector) root.getVector(0);
+      final IntVector strs = (IntVector) root.getVector(1);
+
+      ints.allocateNew(4);
+      ints.setSafe(0, 0);
+      ints.setSafe(1, 1);
+      ints.setSafe(2, 2);
+      ints.setSafe(3, 3);
+      strs.allocateNew(4);
+      strs.setSafe(0, 10);
+      strs.setSafe(1, 11);
+      strs.setSafe(2, 12);
+      strs.setSafe(3, 13);
+      root.setRowCount(4);
+      try (final AdbcStatement stmt = connection.bulkIngest(tableName, BulkIngestMode.CREATE)) {
+        stmt.bind(root);
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery("ALTER TABLE " + tableName + " ALTER COLUMN INTS SET NOT NULL");
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery("ALTER TABLE " + tableName + " ALTER COLUMN INTS2 SET NOT NULL");
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery(

Review Comment:
   Ditto for this, and the other DDL queries



##########
java/driver/jdbc/src/main/java/org/apache/arrow/adbc/driver/jdbc/ObjectMetadataBuilder.java:
##########
@@ -21,11 +21,10 @@
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;

Review Comment:
   nit: I'd prefer to avoid star imports if possible



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm merged pull request #474: feat(java/driver/jdbc): expose constraints in GetObjects

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #474:
URL: https://github.com/apache/arrow-adbc/pull/474


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] github-actions[bot] commented on pull request #474: [JAVA] Fix/Refactor JDBC constraints

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #474:
URL: https://github.com/apache/arrow-adbc/pull/474#issuecomment-1442425906

   :warning: Please follow the [Conventional Commits format in CONTRIBUTING.md](https://github.com/apache/arrow-adbc/blob/main/CONTRIBUTING.md) for PR titles.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] tokoko commented on a diff in pull request #474: feat(java/driver/jdbc): expose constraints in GetObjects

Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #474:
URL: https://github.com/apache/arrow-adbc/pull/474#discussion_r1117741129


##########
java/driver/validation/src/main/java/org/apache/arrow/adbc/driver/testsuite/SqlTestUtil.java:
##########
@@ -69,4 +69,140 @@ public Schema ingestTableIntsStrs(
     }
     return schema;
   }
+
+  /** Load a table with composite primary key */
+  public Schema ingestTableWithConstraints(
+      BufferAllocator allocator, AdbcConnection connection, String tableName) throws Exception {
+    tableName = quirks.caseFoldTableName(tableName);
+    final Schema schema =
+        new Schema(
+            Arrays.asList(
+                Field.notNullable(
+                    quirks.caseFoldColumnName("INTS"), new ArrowType.Int(32, /*signed=*/ true)),
+                Field.nullable(quirks.caseFoldColumnName("INTS2"), new ArrowType.Int(32, true))));
+    try (final VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator)) {
+      final IntVector ints = (IntVector) root.getVector(0);
+      final IntVector strs = (IntVector) root.getVector(1);
+
+      ints.allocateNew(4);
+      ints.setSafe(0, 0);
+      ints.setSafe(1, 1);
+      ints.setSafe(2, 2);
+      ints.setSafe(3, 3);
+      strs.allocateNew(4);
+      strs.setSafe(0, 10);
+      strs.setSafe(1, 11);
+      strs.setSafe(2, 12);
+      strs.setSafe(3, 13);
+      root.setRowCount(4);
+      try (final AdbcStatement stmt = connection.bulkIngest(tableName, BulkIngestMode.CREATE)) {
+        stmt.bind(root);
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery("ALTER TABLE " + tableName + " ALTER COLUMN INTS SET NOT NULL");

Review Comment:
   Resolved. query generation is now in SqlValidationQuirks



##########
java/driver/validation/src/main/java/org/apache/arrow/adbc/driver/testsuite/SqlTestUtil.java:
##########
@@ -69,4 +69,140 @@ public Schema ingestTableIntsStrs(
     }
     return schema;
   }
+
+  /** Load a table with composite primary key */
+  public Schema ingestTableWithConstraints(
+      BufferAllocator allocator, AdbcConnection connection, String tableName) throws Exception {
+    tableName = quirks.caseFoldTableName(tableName);
+    final Schema schema =
+        new Schema(
+            Arrays.asList(
+                Field.notNullable(
+                    quirks.caseFoldColumnName("INTS"), new ArrowType.Int(32, /*signed=*/ true)),
+                Field.nullable(quirks.caseFoldColumnName("INTS2"), new ArrowType.Int(32, true))));
+    try (final VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator)) {
+      final IntVector ints = (IntVector) root.getVector(0);
+      final IntVector strs = (IntVector) root.getVector(1);
+
+      ints.allocateNew(4);
+      ints.setSafe(0, 0);
+      ints.setSafe(1, 1);
+      ints.setSafe(2, 2);
+      ints.setSafe(3, 3);
+      strs.allocateNew(4);
+      strs.setSafe(0, 10);
+      strs.setSafe(1, 11);
+      strs.setSafe(2, 12);
+      strs.setSafe(3, 13);
+      root.setRowCount(4);
+      try (final AdbcStatement stmt = connection.bulkIngest(tableName, BulkIngestMode.CREATE)) {
+        stmt.bind(root);
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery("ALTER TABLE " + tableName + " ALTER COLUMN INTS SET NOT NULL");
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery("ALTER TABLE " + tableName + " ALTER COLUMN INTS2 SET NOT NULL");
+        stmt.executeUpdate();
+      }
+
+      try (final AdbcStatement stmt = connection.createStatement()) {
+        stmt.setSqlQuery(

Review Comment:
   same here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org