lidavidm commented on a change in pull request #11982:
URL: https://github.com/apache/arrow/pull/11982#discussion_r787989846



##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {
+  UNKNOWN_SUBCODE = 0;

Review comment:
       Thank you for adding an UNKNOWN case - that said, can we keep it (and 
the other values) named consistently? This should be SQL_DATE_SUBCODE_UNKNOWN 
(and the others should be SQL_DATE_CODE_DATE, etc.)

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {

Review comment:
       For enum naming style, can we be consistent with the other enum names?

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {
+  UNKNOWN_SUBCODE = 0;
+  SQL_CODE_DATE = 1;
+  SQL_CODE_TIME = 2;
+  SQL_CODE_TIMESTAMP = 3;
+}
+
+enum SQL_DATA_TYPE {
+  UNKNOWN = 0;

Review comment:
       For what it's worth, I'm referencing the Protobuf style guide: 
https://developers.google.com/protocol-buffers/docs/style#enums 
   
   We haven't been following it *too* strictly but I think we can try to be a 
little more consistent.

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {
+  UNKNOWN_SUBCODE = 0;
+  SQL_CODE_DATE = 1;
+  SQL_CODE_TIME = 2;
+  SQL_CODE_TIMESTAMP = 3;
+}
+
+enum SQL_DATA_TYPE {
+  UNKNOWN = 0;

Review comment:
       Similarly, maybe `SQL_CODE_UNKNOWN`, `SQL_CODE_YEAR`, etc.

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {
+  UNKNOWN_SUBCODE = 0;
+  SQL_CODE_DATE = 1;
+  SQL_CODE_TIME = 2;
+  SQL_CODE_TIMESTAMP = 3;
+}
+
+enum SQL_DATA_TYPE {
+  UNKNOWN = 0;
+  SQL_CODE_YEAR = 1;
+  SQL_CODE_MONTH = 2;
+  SQL_CODE_DAY = 3;
+  SQL_CODE_HOUR = 4;
+  SQL_CODE_MINUTE = 5;
+  SQL_CODE_SECOND = 6;
+  SQL_CODE_YEAR_TO_MONTH = 7;
+  SQL_CODE_DAY_TO_HOUR = 8;
+  SQL_CODE_DAY_TO_MINUTE = 9;
+  SQL_CODE_DAY_TO_SECOND = 10;
+  SQL_CODE_HOUR_TO_MINUTE = 11;
+  SQL_CODE_HOUR_TO_SECOND = 12;
+  SQL_CODE_MINUTE_TO_SECOND = 13;
+  SQL_INTERVAL_YEAR = 101;
+  SQL_INTERVAL_MONTH = 102;
+  SQL_INTERVAL_DAY = 103;
+  SQL_INTERVAL_HOUR = 104;
+  SQL_INTERVAL_MINUTE = 105;
+  SQL_INTERVAL_SECOND = 106;
+  SQL_INTERVAL_YEAR_TO_MONTH = 107;
+  SQL_INTERVAL_DAY_TO_HOUR = 108;
+  SQL_INTERVAL_DAY_TO_MINUTE = 109;
+  SQL_INTERVAL_DAY_TO_SECOND = 110;
+  SQL_INTERVAL_HOUR_TO_MINUTE = 111;
+  SQL_INTERVAL_HOUR_TO_SECOND = 112;
+  SQL_INTERVAL_MINUTE_TO_SECOND = 113;
+}
+
+enum Nullable {
+  /**
+   * Indicates that the fields does not allow the use of null values.
+   */
+  TYPE_NO_NULLS = 0;
+
+  /**
+   * Indicates that the fields allow the use of null values.
+   */
+  TYPE_NULLABLE = 1;
+
+  /**
+   * Indicates that nullability of the fields can not be determined.
+   */
+  TYPE_NULLABLE_UNKNOWN = 2;
+}
+
+enum Searchable {
+  /**
+   * Indicates that column can not be used in a WHERE clause.
+   */
+  TYPE_PRED_NONE = 0;
+
+  /**
+   * Indicates that the column can be used in a WHERE clause if it is using a
+   * LIKE predicate.
+   */
+  TYPE_PRED_CHAR = 1;
+
+  /**
+   * Indicates that the column can be used in a WHERE clause using other 
predicates
+   * except for LIKE.
+   *
+   * - Allowed operators: comparison, quantified comparison, BETWEEN,
+   *                      DISTINCT, IN, MATCH, and UNIQUE.
+   */
+  TYPE_PRED_BASIC = 2;
+
+  /**
+   * Indicates that the column can be used in a WHERE clause using any 
operator.
+   */
+  TYPE_SEARCHABLE = 3;
+}
+
+/*
+ * Represents a request to retrieve information about data type supported on a 
Flight SQL enabled backend.
+ * Used in the command member of FlightDescriptor for the following RPC calls:
+ *  - GetSchema: return the schema of the query.
+ *  - GetFlightInfo: execute the catalog metadata request.
+ *
+ * The returned schema will be:
+ * <
+ *   type_name: utf8 not null (The name of the data type, for example: 
VARCHAR, INTEGER, etc),
+ *   data_type: int not null (The SQL data type),
+ *   column_size: int (The maximum size supported by that column.
+ *                     In case of numeric types, this represents the maximum 
precision.
+ *                     In case of string types, this represents the character 
length.
+ *                     In case of datetime data types, this represents the 
length in characters of the string representation.
+ *                     NULL is returned for data types where column size is 
not applicable.),
+ *   literal_prefix: utf8 (Character or characters used to prefix a literal, 
NULL is returned for
+ *                         data types where a literal prefix is not 
applicable.),
+ *   literal_suffix: utf8 (Character or characters used to terminate a literal,
+ *                         NULL is returned for data types where a literal 
suffix is not applicable.),
+ *   create_params: list<utf8 not null>
+                          (A list of keywords, separated by commas, 
corresponding

Review comment:
       This is no longer separated by commas.

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,69 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+/*
+ * Represents a request to retrieve information about data type supported ona 
Flight SQL enabled backend.
+ * Used in the command member of FlightDescriptor for the following RPC calls:
+ *  - GetSchema: return the schema of the query.
+ *  - GetFlightInfo: execute the catalog metadata request.
+ *
+ * The returned schema will be:
+ * <
+ *   type_name: utf8 not null (The name of the data type, for example: 
VARCHAR, INTEGER, etc),
+ *   data_type: int not null (The SQL data type),
+ *   column_size: int (The maximum column size that the server supports for 
this data type.
+ *                     For numeric data, this is the maximum precision.
+ *                     For string data, this is the length in characters.
+ *                     For datetime data types, this is the length in 
characters of the string representation.
+ *                     NULL is returned for data types where column size is 
not applicable.),
+ *   literal_prefix: utf8 (Character or characters used to prefix a literal, 
NULL is returned for
+ *                         data types where a literal prefix is not 
applicable.),
+ *   literal_suffix: utf8 (Character or characters used to terminate a literal,
+ *                         NULL is returned for data types where a literal 
suffix is not applicable.),
+ *   create_params: utf8 (A list of keywords, separated by commas, 
corresponding
+ *                        to each parameter that the application may specify 
in parentheses
+ *                        when using the name that is returned in the 
TYPE_NAME field.
+ *                        NULL is returned if there are no parameters for the 
data type definition.),
+ *   nullable: int not null (Shows if the data type accepts a NULL value),

Review comment:
       Thanks. Can we explicitly reference the corresponding enum in the 
description? (And ditto for other such fields.)

##########
File path: cpp/src/arrow/flight/sql/server.cc
##########
@@ -757,6 +788,29 @@ std::shared_ptr<Schema> SqlSchema::GetSqlInfoSchema() {
                               false)});
 }
 
+std::shared_ptr<Schema> SqlSchema::GetTypeInfoSchema() {
+  return arrow::schema({
+      field("type_name", utf8(), false),
+      field("data_type", int32(), false),
+      field("column_size", int32()),
+      field("literal_prefix", utf8()),
+      field("literal_suffix", utf8()),
+      field("create_params", list(utf8()), false),

Review comment:
       Maybe this should be `field("create_params", list(field("item", utf8(), 
false)))`?

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {
+  UNKNOWN_SUBCODE = 0;
+  SQL_CODE_DATE = 1;
+  SQL_CODE_TIME = 2;
+  SQL_CODE_TIMESTAMP = 3;
+}
+
+enum SQL_DATA_TYPE {
+  UNKNOWN = 0;
+  SQL_CODE_YEAR = 1;
+  SQL_CODE_MONTH = 2;
+  SQL_CODE_DAY = 3;
+  SQL_CODE_HOUR = 4;
+  SQL_CODE_MINUTE = 5;
+  SQL_CODE_SECOND = 6;
+  SQL_CODE_YEAR_TO_MONTH = 7;
+  SQL_CODE_DAY_TO_HOUR = 8;
+  SQL_CODE_DAY_TO_MINUTE = 9;
+  SQL_CODE_DAY_TO_SECOND = 10;
+  SQL_CODE_HOUR_TO_MINUTE = 11;
+  SQL_CODE_HOUR_TO_SECOND = 12;
+  SQL_CODE_MINUTE_TO_SECOND = 13;
+  SQL_INTERVAL_YEAR = 101;
+  SQL_INTERVAL_MONTH = 102;
+  SQL_INTERVAL_DAY = 103;
+  SQL_INTERVAL_HOUR = 104;
+  SQL_INTERVAL_MINUTE = 105;
+  SQL_INTERVAL_SECOND = 106;
+  SQL_INTERVAL_YEAR_TO_MONTH = 107;
+  SQL_INTERVAL_DAY_TO_HOUR = 108;
+  SQL_INTERVAL_DAY_TO_MINUTE = 109;
+  SQL_INTERVAL_DAY_TO_SECOND = 110;
+  SQL_INTERVAL_HOUR_TO_MINUTE = 111;
+  SQL_INTERVAL_HOUR_TO_SECOND = 112;
+  SQL_INTERVAL_MINUTE_TO_SECOND = 113;
+}
+
+enum Nullable {
+  /**
+   * Indicates that the fields does not allow the use of null values.
+   */
+  TYPE_NO_NULLS = 0;

Review comment:
       Similarly, while ODBC/JDBC might have certain names, we don't have to 
stick to those names 1:1 right? This could be NULLABILITY_NO_NULLS, 
NULLABILITY_NULLABLE, NULLABILITY_UNKNOWN (and usually UNKNOWN should be the 
zero value, unless the exact value is important here)

##########
File path: format/FlightSql.proto
##########
@@ -867,6 +867,151 @@ enum SqlSupportsConvert {
   SQL_CONVERT_VARCHAR = 19;
 }
 
+enum DATE_SUBCODE {
+  UNKNOWN_SUBCODE = 0;

Review comment:
       Or I guess `SQL_DATETIME_SUBCODE_UNKNOWN`  since the field is 
`datetime_subcode` below.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to