You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/09/09 08:15:19 UTC
[GitHub] [arrow] igor-suhorukov opened a new pull request, #14081: Propagate table/columns comments into Arrow Schema
igor-suhorukov opened a new pull request, #14081:
URL: https://github.com/apache/arrow/pull/14081
Allow user to provide comment in Arrow Schema from JdbcToArrowConfig . It will be very useful metadata in real life (medium to large scale project) for documentation and maintenance topics. Apache Spark code use "comment" key for such metadata, so this looks like reasonable default name for metadata in Arrow schema too
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967244098
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/Constants.java:
##########
@@ -27,5 +27,6 @@ private Constants() {}
public static final String SQL_TABLE_NAME_KEY = "SQL_TABLE_NAME";
public static final String SQL_COLUMN_NAME_KEY = "SQL_COLUMN_NAME";
public static final String SQL_TYPE_KEY = "SQL_TYPE";
+ public static final String COMMENT = "comment";
Review Comment:
I prefer "comment" as it much more easy to don't introduce new names for de facto interoperability with Apache Spark https://github.com/apache/spark/blob/e83aedd0f072ce18d5542b61eb40e2df1b83bb50/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructField.scala#L85
Roundtrip conversion with org.apache.spark.sql.types.StructType/StructField and huge existing ecosystem around (AWS Glue etc)
--
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] lidavidm commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967252660
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
(Of course, you could probably add the metadata during reading as well…but since we already offer some options, I guess we may as well have a formal API.)
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967250982
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Good idea, @lidavidm!
Do you mean change new JdbcToArrowConfig fields to :
private final Map<String,String> schemaMetadata;
private final Map<Integer, Map<String,String>> columnMetadataByColumnIndex;
? To allow user provide any additional metadata for schema/column level? For me such approach is super rational - it allows propagate additional metadata for example SRID related to geometry /Spatial Reference Systems
It also allow to remove hard coded name String COMMENT = "comment" from source and move naming responsibility to developer side
--
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] lidavidm commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1245678104
The path can be added here: https://github.com/apache/arrow/blob/master/dev/release/rat_exclude_files.txt
--
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] igor-suhorukov commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1245658676
Ok, done license header with h2 sql file. But is comment applicable to json expected dataset? I don't think that it possible for json files
--
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] lidavidm commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967247379
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/Constants.java:
##########
@@ -27,5 +27,6 @@ private Constants() {}
public static final String SQL_TABLE_NAME_KEY = "SQL_TABLE_NAME";
public static final String SQL_COLUMN_NAME_KEY = "SQL_COLUMN_NAME";
public static final String SQL_TYPE_KEY = "SQL_TYPE";
+ public static final String COMMENT = "comment";
Review Comment:
Ah, if this is for spark compatibility that's good to know.
That said - I think my comment below makes more sense then, what do you think? We should just let you add whatever metadata instead of hardcoding particular fields
--
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] github-actions[bot] commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1241689957
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
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] lidavidm commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967252055
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Yes! That way you don't have to submit a new PR every time there's new metadata :)
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967258562
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Sure! I'll do it on Monday. Thank you for your assistance and good idea! Have a nice weekend @lidavidm
--
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] lidavidm merged pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm merged PR #14081:
URL: https://github.com/apache/arrow/pull/14081
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967250982
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Good idea, @lidavidm!
Do you mean change new JdbcToArrowConfig fields to :
private final Map<String,String> schemaMetadata;
private final Map<Integer, Map<String,String>> columnMetadataByColumnIndex;
? To allow user provide any additional metadata for schema/column level? For me such approach is super rational - it allows propagate additional metadata for example SRID related to geometry /Spatial Reference Systems
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967245996
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/Constants.java:
##########
@@ -27,5 +27,6 @@ private Constants() {}
public static final String SQL_TABLE_NAME_KEY = "SQL_TABLE_NAME";
public static final String SQL_COLUMN_NAME_KEY = "SQL_COLUMN_NAME";
public static final String SQL_TYPE_KEY = "SQL_TYPE";
+ public static final String COMMENT = "comment";
Review Comment:
>>Also, since the JDBC name for this is REMARKS, should we stay consistent with that?
No, I not think so as it internal JDBC naming, but Arrow target following Spark/Hadoop but not JDBC
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/Constants.java:
##########
@@ -27,5 +27,6 @@ private Constants() {}
public static final String SQL_TABLE_NAME_KEY = "SQL_TABLE_NAME";
public static final String SQL_COLUMN_NAME_KEY = "SQL_COLUMN_NAME";
public static final String SQL_TYPE_KEY = "SQL_TYPE";
+ public static final String COMMENT = "comment";
Review Comment:
>Also, since the JDBC name for this is REMARKS, should we stay consistent with that?
No, I not think so as it internal JDBC naming, but Arrow target following Spark/Hadoop but not JDBC
--
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] github-actions[bot] commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1241681205
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW
Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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] lidavidm commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967105833
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/Constants.java:
##########
@@ -27,5 +27,6 @@ private Constants() {}
public static final String SQL_TABLE_NAME_KEY = "SQL_TABLE_NAME";
public static final String SQL_COLUMN_NAME_KEY = "SQL_COLUMN_NAME";
public static final String SQL_TYPE_KEY = "SQL_TYPE";
+ public static final String COMMENT = "comment";
Review Comment:
Also, since the JDBC name for this is REMARKS, should we stay consistent with that?
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Ah, for the other metadata, this method automatically extracts the metadata values and adds them here. But for REMARKS, it's non-trivial to extract it from the result set so it probably shouldn't be done automatically.
At this point, I wonder if the API shouldn't just be: allow specifying extra metadata to attach to the schema and to each column, instead of special casing a single metadata value?
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/Constants.java:
##########
@@ -27,5 +27,6 @@ private Constants() {}
public static final String SQL_TABLE_NAME_KEY = "SQL_TABLE_NAME";
public static final String SQL_COLUMN_NAME_KEY = "SQL_COLUMN_NAME";
public static final String SQL_TYPE_KEY = "SQL_TYPE";
+ public static final String COMMENT = "comment";
Review Comment:
I think this should have the `SQL_` prefix (in both name and value) for consistency with the other constants
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967633038
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
To Apache Spark schema conversion result for schema from org.apache.arrow.adapter.jdbc.JdbcToArrowCommentMetadataTest#schemaCommentWithDatabaseMetadata
structType.toDDL(): ID BIGINT NOT NULL COMMENT 'Record identifier',NAME STRING COMMENT 'Name of record',COLUMN1 BOOLEAN,COLUMNN INT COMMENT 'Informative description of columnN'
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967254924
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Cool, so I'll change implementation to more common approach but test case will be the same with "comment". Is it OK for you?
For me test - like code snippets for developers, somebody can implement complex DB metadata gathering base on the same idea. For PostgreSQL driver required special wrapper around public class ResultSetMetaData to propagate right metadata to existing jdbc->arrow bridge
--
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] lidavidm commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967255845
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Yes, we can keep the test case.
It may help to also note in the test case that COMMENT is what Spark uses - I wasn't aware of that context
--
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] github-actions[bot] commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1241689934
https://issues.apache.org/jira/browse/ARROW-17631
--
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] lidavidm commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1245613072
I'll take a second look when I can but it may be a few days
--
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] igor-suhorukov commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1245765820
json as exception already 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
[GitHub] [arrow] ursabot commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1247338976
Benchmark runs are scheduled for baseline = fac08404aa7018e6bcab515125ca99856d624d89 and contender = 9d33df19d9a98df5caf134f2792e5c81bca90ae3. 9d33df19d9a98df5caf134f2792e5c81bca90ae3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:25.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/ba5bb5653e824ab4839f9c6f85df62b5...f675e7eb95534b20bed0c7499e85c55e/)
[Finished :arrow_down:0.2% :arrow_up:0.07%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/5fb74e0ce5634305854693668b40a46c...9fc41f3e7ada43edba60ca47fec5abc0/)
[Failed :arrow_down:0.28% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/17ec0ca4d4e948129ba87c3ecd8f4814...bab02be1fe174735b08064e46e9e0fec/)
[Finished :arrow_down:0.11% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/3e432db2acda497baf5f8ac0b993c22f...c980858bf4444af9874be3423864918e/)
Buildkite builds:
[Finished] [`9d33df19` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1474)
[Finished] [`9d33df19` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1490)
[Failed] [`9d33df19` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1474)
[Finished] [`9d33df19` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1487)
[Finished] [`fac08404` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1473)
[Finished] [`fac08404` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1489)
[Failed] [`fac08404` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1473)
[Finished] [`fac08404` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1486)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] igor-suhorukov commented on a diff in pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on code in PR #14081:
URL: https://github.com/apache/arrow/pull/14081#discussion_r967254924
##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowCommentMetadataTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adapter.jdbc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.types.pojo.Schema;
+import org.apache.arrow.vector.util.ObjectMapperFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JdbcToArrowCommentMetadataTest {
+
+ private final ObjectWriter schemaSerializer = ObjectMapperFactory.newObjectMapper().writerWithDefaultPrettyPrinter();
+ private Connection conn = null;
+
+ /**
+ * This method creates Connection object and DB table and also populate data into table for test.
+ *
+ * @throws SQLException on error
+ * @throws ClassNotFoundException on error
+ */
+ @Before
+ public void setUp() throws SQLException, ClassNotFoundException {
+ String url = "jdbc:h2:mem:JdbcToArrowTest?characterEncoding=UTF-8;INIT=runscript from 'classpath:/h2/comment.sql'";
+ String driver = "org.h2.Driver";
+ Class.forName(driver);
+ conn = DriverManager.getConnection(url);
+ }
+
+ @After
+ public void tearDown() throws SQLException {
+ if (conn != null) {
+ conn.close();
+ conn = null;
+ }
+ }
+
+ @Test
+ public void schemaComment() throws Exception {
+ boolean includeMetadata = false;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithComments.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ @Test
+ public void schemaCommentWithDatabaseMetadata() throws Exception {
+ boolean includeMetadata = true;
+ String schemaJson = schemaSerializer.writeValueAsString(getSchemaWithCommentFromQuery(includeMetadata));
+ String expectedSchema = getExpectedSchema("/h2/expectedSchemaWithCommentsAndJdbcMeta.json");
+ assertThat(schemaJson).isEqualTo(expectedSchema);
+ }
+
+ private Schema getSchemaWithCommentFromQuery(boolean includeMetadata) throws SQLException {
+ DatabaseMetaData metaData = conn.getMetaData();
+ try (Statement statement = conn.createStatement()) {
+ try (ResultSet resultSet = statement.executeQuery("select * from table1")) {
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ Map<Integer, String> columnCommentByColumnIndex = getColumnComments(metaData, resultSetMetaData);
+
+ String tableName = getTableNameFromResultSetMetaData(resultSetMetaData);
+ String tableComment = getTableComment(metaData, tableName);
+ JdbcToArrowConfig config = new JdbcToArrowConfigBuilder()
+ .setAllocator(new RootAllocator()).setSchemaComment(tableComment)
+ .setColumnCommentByColumnIndex(columnCommentByColumnIndex).setIncludeMetadata(includeMetadata).build();
+ return JdbcToArrowUtils.jdbcToArrowSchema(resultSetMetaData, config);
Review Comment:
Cool, so I'll change implementation to more common approach but test case will be the same with "comment". Is it OK for you?
For me test - like code snippets for developers, somebody can implement complex DB metadata gathering base on the same idea
--
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] lidavidm commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1245612628
Hi, sorry, it seems there's still a lint error - looks like the newly added files need the license header at top, something like this https://github.com/apache/arrow/blob/8bf60b5deaa15a6e301221b57b419c9599e313b0/java/adapter/jdbc/src/test/resources/h2/test1_all_datatypes_h2.yml#L1-L10
--
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] igor-suhorukov commented on pull request #14081: ARROW-17631: [Java] Propagate table/columns comments into Arrow Schema
Posted by GitBox <gi...@apache.org>.
igor-suhorukov commented on PR #14081:
URL: https://github.com/apache/arrow/pull/14081#issuecomment-1245610147
Hello @lidavidm
Is code ok after rework?
--
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