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 2021/12/20 18:21:56 UTC
[GitHub] [arrow] lidavidm commented on a change in pull request #11999: [JAVA] Add missing metadata on Arrow schemas returned by Flight SQL
lidavidm commented on a change in pull request #11999:
URL: https://github.com/apache/arrow/pull/11999#discussion_r772574683
##########
File path: format/FlightSql.proto
##########
@@ -55,7 +55,7 @@ message CommandGetSqlInfo {
* Initially, Flight SQL will support the following information types:
* - Server Information - Range [0-500)
* - Syntax Information - Range [500-1000)
- * Range [0-10,000) is reserved for defaults (see SqlInfo enum for default options).
+ * Range [0-10,000] is reserved for defaults (see SqlInfo enum for default options).
Review comment:
This seems wrong if custom options start at 10_000.
##########
File path: format/FlightSql.proto
##########
@@ -933,6 +933,16 @@ message CommandGetDbSchemas {
* [optional] table_schema: bytes not null (schema of the table as described in Schema.fbs::Schema,
* it is serialized as an IPC message.)
* >
+ * Fields on table_schema may contain the following metadata:
+ * - CATALOG_NAME - Table's catalog name
+ * - SCHEMA_NAME - Table's schema name
+ * - TABLE_NAME - Table name
+ * - PRECISION - Column precision/size
+ * - SCALE - Column scale/decimal digits
+ * - IS_AUTO_INCREMENT - "1" if column is auto incremented, "0" otherwise.
+ * - IS_CASE_SENSITIVE - "1" if column is case sensitive, "0" otherwise.
+ * - IS_READ_ONLY - "1" if column is read only, "0" otherwise.
+ * - IS_SEARCHABLE - "1" if column is searchable, "0" otherwise.
Review comment:
Can we namespace these values? e.g. `arrow.flight.sql.catalog_name`.
Or actually, in analogy with [extension types](https://arrow.apache.org/docs/format/Columnar.html#extension-types) it should be something like `ARROW:FLIGHT:SQL:catalog_name`
##########
File path: java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlColumnMetadata.java
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.flight.sql;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Metadata for a column in a Flight SQL query.
+ *
+ * This can be used with FlightSqlClient to access column's metadata contained in schemas returned
+ * by GetTables and query execution as follows:
+ * <pre>
+ * FlightSqlColumnMetadata metadata = new FlightSqlColumnMetadata(field.getMetadata());
+ * Integer precision = metadata.getPrecision();
+ * </pre>
+ *
+ * FlightSqlProducer can use this to set metadata on a column in a schema as follows:
+ * <pre>
+ * FlightSqlColumnMetadata metadata = new FlightSqlColumnMetadata.Builder()
+ * .precision(10)
+ * .scale(5)
+ * .build();
+ * Field field = new Field("column", new FieldType(..., metadata.getMetadataMap()), null);
+ * </pre>
+ */
+public class FlightSqlColumnMetadata {
+
+ private static final String CATALOG_NAME = "CATALOG_NAME";
+ private static final String SCHEMA_NAME = "SCHEMA_NAME";
+ private static final String TABLE_NAME = "TABLE_NAME";
+ private static final String PRECISION = "PRECISION";
+ private static final String SCALE = "SCALE";
+ private static final String IS_AUTO_INCREMENT = "IS_AUTO_INCREMENT";
+ private static final String IS_CASE_SENSITIVE = "IS_CASE_SENSITIVE";
+ private static final String IS_READ_ONLY = "IS_READ_ONLY";
+ private static final String IS_SEARCHABLE = "IS_SEARCHABLE";
+
+ private static final String BOOLEAN_TRUE_STR = "1";
+ private static final String BOOLEAN_FALSE_STR = "0";
+
+ private final Map<String, String> metadataMap;
+
+ /**
+ * Creates a new instance of FlightSqlColumnMetadata.
+ */
+ public FlightSqlColumnMetadata(Map<String, String> metadataMap) {
+ this.metadataMap = metadataMap;
+ }
+
+ /**
+ * Returns the metadata map.
+ * @return The metadata map.
+ */
+ public Map<String, String> getMetadataMap() {
+ return metadataMap;
Review comment:
Similarly, this might want to wrap in `Collections.unmodifiableMap`.
##########
File path: java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlColumnMetadata.java
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.flight.sql;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Metadata for a column in a Flight SQL query.
+ *
+ * This can be used with FlightSqlClient to access column's metadata contained in schemas returned
+ * by GetTables and query execution as follows:
+ * <pre>
+ * FlightSqlColumnMetadata metadata = new FlightSqlColumnMetadata(field.getMetadata());
+ * Integer precision = metadata.getPrecision();
+ * </pre>
+ *
+ * FlightSqlProducer can use this to set metadata on a column in a schema as follows:
+ * <pre>
+ * FlightSqlColumnMetadata metadata = new FlightSqlColumnMetadata.Builder()
+ * .precision(10)
+ * .scale(5)
+ * .build();
+ * Field field = new Field("column", new FieldType(..., metadata.getMetadataMap()), null);
+ * </pre>
+ */
+public class FlightSqlColumnMetadata {
+
+ private static final String CATALOG_NAME = "CATALOG_NAME";
+ private static final String SCHEMA_NAME = "SCHEMA_NAME";
+ private static final String TABLE_NAME = "TABLE_NAME";
+ private static final String PRECISION = "PRECISION";
+ private static final String SCALE = "SCALE";
+ private static final String IS_AUTO_INCREMENT = "IS_AUTO_INCREMENT";
+ private static final String IS_CASE_SENSITIVE = "IS_CASE_SENSITIVE";
+ private static final String IS_READ_ONLY = "IS_READ_ONLY";
+ private static final String IS_SEARCHABLE = "IS_SEARCHABLE";
+
+ private static final String BOOLEAN_TRUE_STR = "1";
+ private static final String BOOLEAN_FALSE_STR = "0";
+
+ private final Map<String, String> metadataMap;
+
+ /**
+ * Creates a new instance of FlightSqlColumnMetadata.
+ */
+ public FlightSqlColumnMetadata(Map<String, String> metadataMap) {
+ this.metadataMap = metadataMap;
Review comment:
Should this make a copy of the map to guard against mutation?
##########
File path: format/FlightSql.proto
##########
@@ -933,6 +933,16 @@ message CommandGetDbSchemas {
* [optional] table_schema: bytes not null (schema of the table as described in Schema.fbs::Schema,
* it is serialized as an IPC message.)
* >
+ * Fields on table_schema may contain the following metadata:
+ * - CATALOG_NAME - Table's catalog name
+ * - SCHEMA_NAME - Table's schema name
+ * - TABLE_NAME - Table name
+ * - PRECISION - Column precision/size
+ * - SCALE - Column scale/decimal digits
+ * - IS_AUTO_INCREMENT - "1" if column is auto incremented, "0" otherwise.
+ * - IS_CASE_SENSITIVE - "1" if column is case sensitive, "0" otherwise.
+ * - IS_READ_ONLY - "1" if column is read only, "0" otherwise.
+ * - IS_SEARCHABLE - "1" if column is searchable, "0" otherwise.
Review comment:
Can we also define what "searchable" means in this context?
##########
File path: format/FlightSql.proto
##########
@@ -933,6 +933,16 @@ message CommandGetDbSchemas {
* [optional] table_schema: bytes not null (schema of the table as described in Schema.fbs::Schema,
* it is serialized as an IPC message.)
* >
+ * Fields on table_schema may contain the following metadata:
+ * - CATALOG_NAME - Table's catalog name
+ * - SCHEMA_NAME - Table's schema name
+ * - TABLE_NAME - Table name
+ * - PRECISION - Column precision/size
+ * - SCALE - Column scale/decimal digits
+ * - IS_AUTO_INCREMENT - "1" if column is auto incremented, "0" otherwise.
+ * - IS_CASE_SENSITIVE - "1" if column is case sensitive, "0" otherwise.
+ * - IS_READ_ONLY - "1" if column is read only, "0" otherwise.
+ * - IS_SEARCHABLE - "1" if column is searchable, "0" otherwise.
Review comment:
Assuming that if a particular metadata is not known or not supported, it should just be omitted?
##########
File path: format/FlightSql.proto
##########
@@ -933,6 +933,16 @@ message CommandGetDbSchemas {
* [optional] table_schema: bytes not null (schema of the table as described in Schema.fbs::Schema,
* it is serialized as an IPC message.)
* >
+ * Fields on table_schema may contain the following metadata:
+ * - CATALOG_NAME - Table's catalog name
+ * - SCHEMA_NAME - Table's schema name
Review comment:
"DB_SCHEMA_NAME - database schema name" for consistency?
--
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