Skip to content

Commit

Permalink
Flight SQL Ratification Based On Community Feedback #6 (apache#94)
Browse files Browse the repository at this point in the history 
* Refactored FlightSql Statement Constant names

* Defined non-nullable parameters for FlightSql proto

* Resolved minimal checkstyle issues

* Added further documentation for catalog and schema

* Refactored FlightSql proto comments to include more information

* Added Field/FieldType notNullable methods

* Refactored FlightSqlClient and FlightSqlExample to leverage Field notNullable method

* Removed opaque query warning from FlightSql proto

* Added the optional tag for the returned schema of getTables to proto
  • Loading branch information
@vfraga
vfraga committed Oct 28, 2021
1 parent 7805e71 commit 26482f6
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 77 deletions.
98 changes: 63 additions & 35 deletions format/FlightSql.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ package arrow.flight.protocol.sql;
*
* The returned schema will be:
* <
* info_name: uint32,
* info_name: uint32 not null,
* value: dense_union<string_value: string, int_value: int32, bigint_value: int64, int32_bitmask: int32>
* >
* where there is one row per requested piece of metadata information.
Expand Down Expand Up @@ -156,7 +156,7 @@ enum SqlInfo {
*
* The returned schema will be:
* <
* catalog_name: utf8
* catalog_name: utf8 not null
* >
* The returned data should be ordered by catalog_name.
*/
Expand All @@ -173,16 +173,17 @@ message CommandGetCatalogs {
* The returned schema will be:
* <
* catalog_name: utf8,
* schema_name: utf8
* schema_name: utf8 not null
* >
* The returned data should be ordered by catalog_name, then schema_name.
*/
message CommandGetSchemas {
option (experimental) = true;

/*
* Specifies the Catalog to search for schemas.
* If omitted, then all catalogs are searched.
* Specifies the Catalog to search for the tables.
* An empty string retrieves those without a catalog.
* If omitted the catalog name should not be used to narrow the search.
*/
google.protobuf.StringValue catalog = 1;

Expand All @@ -206,18 +207,20 @@ message CommandGetSchemas {
* <
* catalog_name: utf8,
* schema_name: utf8,
* table_name: utf8,
* table_type: utf8,
* table_schema: bytes (schema of the table as described in Schema.fbs::Schema, it is serialized as an IPC message.)
* table_name: utf8 not null,
* table_type: utf8 not null,
* [optional] table_schema: bytes not null (schema of the table as described in Schema.fbs::Schema,
* it is serialized as an IPC message.)
* >
* The returned data should be ordered by catalog_name, schema_name, table_name, then table_type.
* The returned data should be ordered by catalog_name, schema_name, table_name, then table_type, followed by table_schema if requested.
*/
message CommandGetTables {
option (experimental) = true;

/*
* Specifies the Catalog to search for the tables.
* If omitted, then all catalogs are searched.
* An empty string retrieves those without a catalog.
* If omitted the catalog name should not be used to narrow the search.
*/
google.protobuf.StringValue catalog = 1;

Expand Down Expand Up @@ -254,7 +257,7 @@ message CommandGetTables {
*
* The returned schema will be:
* <
* table_type: utf8
* table_type: utf8 not null
* >
* The returned data should be ordered by table_type.
*/
Expand Down Expand Up @@ -282,14 +285,22 @@ message CommandGetTableTypes {
message CommandGetPrimaryKeys {
option (experimental) = true;

// Specifies the catalog to search for the table.
/*
* Specifies the catalog to search for the table.
* An empty string retrieves those without a catalog.
* If omitted the catalog name should not be used to narrow the search.
*/
google.protobuf.StringValue catalog = 1;

// Specifies the schema to search for the table.
/*
* Specifies the schema to search for the table.
* An empty string retrieves those without a schema.
* If omitted the schema name should not be used to narrow the search.
*/
google.protobuf.StringValue schema = 2;

// Specifies the table to get the primary keys for.
google.protobuf.StringValue table = 3;
string table = 3;
}

/*
Expand All @@ -303,28 +314,36 @@ message CommandGetPrimaryKeys {
* <
* pk_catalog_name: utf8,
* pk_schema_name: utf8,
* pk_table_name: utf8,
* pk_column_name: utf8,
* pk_table_name: utf8 not null,
* pk_column_name: utf8 not null,
* fk_catalog_name: utf8,
* fk_schema_name: utf8,
* fk_table_name: utf8,
* fk_column_name: utf8,
* key_sequence: int,
* fk_table_name: utf8 not null,
* fk_column_name: utf8 not null,
* key_sequence: int not null,
* fk_key_name: utf8,
* pk_key_name: utf8,
* update_rule: uint1,
* delete_rule: uint1
* update_rule: uint1 not null,
* delete_rule: uint1 not null
* >
* The returned data should be ordered by fk_catalog_name, fk_schema_name, fk_table_name, fk_key_name, then key_sequence.
* update_rule and delete_rule returns a byte that is equivalent to actions declared on UpdateDeleteRules enum.
*/
message CommandGetExportedKeys {
option (experimental) = true;

// Specifies the catalog to search for the foreign key table.
/*
* Specifies the catalog to search for the foreign key table.
* An empty string retrieves those without a catalog.
* If omitted the catalog name should not be used to narrow the search.
*/
google.protobuf.StringValue catalog = 1;

// Specifies the schema to search for the foreign key table.
/*
* Specifies the schema to search for the foreign key table.
* An empty string retrieves those without a schema.
* If omitted the schema name should not be used to narrow the search.
*/
google.protobuf.StringValue schema = 2;

// Specifies the foreign key table to get the foreign keys for.
Expand All @@ -349,17 +368,17 @@ enum UpdateDeleteRules {
* <
* pk_catalog_name: utf8,
* pk_schema_name: utf8,
* pk_table_name: utf8,
* pk_column_name: utf8,
* pk_table_name: utf8 not null,
* pk_column_name: utf8 not null,
* fk_catalog_name: utf8,
* fk_schema_name: utf8,
* fk_table_name: utf8,
* fk_column_name: utf8,
* key_sequence: int,
* fk_table_name: utf8 not null,
* fk_column_name: utf8 not null,
* key_sequence: int not null,
* fk_key_name: utf8,
* pk_key_name: utf8,
* update_rule: uint1,
* delete_rule: uint1
* update_rule: uint1 not null,
* delete_rule: uint1 not null
* >
* The returned data should be ordered by pk_catalog_name, pk_schema_name, pk_table_name, pk_key_name, then key_sequence.
* update_rule and delete_rule returns a byte that is equivalent to actions:
Expand All @@ -372,10 +391,18 @@ enum UpdateDeleteRules {
message CommandGetImportedKeys {
option (experimental) = true;

// Specifies the catalog to search for the primary key table.
/*
* Specifies the catalog to search for the primary key table.
* An empty string retrieves those without a catalog.
* If omitted the catalog name should not be used to narrow the search.
*/
google.protobuf.StringValue catalog = 1;

// Specifies the schema to search for the primary key table.
/*
* Specifies the schema to search for the primary key table.
* An empty string retrieves those without a schema.
* If omitted the schema name should not be used to narrow the search.
*/
google.protobuf.StringValue schema = 2;

// Specifies the primary key table to get the foreign keys for.
Expand All @@ -391,12 +418,15 @@ message ActionCreatePreparedStatementRequest {
option (experimental) = true;

// The valid SQL string to create a prepared statement for.
// The query should be treated as an opaque value, that is, clients should not attempt to parse this.
string query = 1;
}

/*
* Wrap the result of a "GetPreparedStatement" action.
*
* The resultant PreparedStatement can be closed either:
* - Manually, through the "ClosePreparedStatement" action;
* - Automatically, by a server timeout.
*/
message ActionCreatePreparedStatementResult {
option (experimental) = true;
Expand Down Expand Up @@ -437,7 +467,6 @@ message CommandStatementQuery {
option (experimental) = true;

// The SQL syntax.
// The query should be treated as an opaque value, that is, clients should not attempt to parse this.
string query = 1;
}

Expand Down Expand Up @@ -473,7 +502,6 @@ message CommandStatementUpdate {
option (experimental) = true;

// The SQL syntax.
// The query should be treated as an opaque value, that is, clients should not attempt to parse this.
string query = 1;
}

Expand Down
10 changes: 5 additions & 5 deletions java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,9 @@ public FlightInfo getPrimaryKeys(final String catalog, final String schema,
builder.setSchema(StringValue.newBuilder().setValue(schema).build());
}

if (table != null) {
builder.setTable(StringValue.newBuilder().setValue(table).build());
}
Objects.requireNonNull(table);
builder.setTable(table).build();

final FlightDescriptor descriptor = FlightDescriptor.command(Any.pack(builder.build()).toByteArray());
return client.getInfo(descriptor, options);
}
Expand Down Expand Up @@ -376,7 +376,7 @@ public static class PreparedStatement implements AutoCloseable {
public PreparedStatement(final FlightClient client, final String sql, final CallOption... options) {
this.client = client;
final Action action = new Action(
FlightSqlUtils.FLIGHT_SQL_CREATEPREPAREDSTATEMENT.getType(),
FlightSqlUtils.FLIGHT_SQL_CREATE_PREPARED_STATEMENT.getType(),
Any.pack(ActionCreatePreparedStatementRequest
.newBuilder()
.setQuery(sql)
Expand Down Expand Up @@ -530,7 +530,7 @@ public void close(final CallOption... options) {
}
isClosed = true;
final Action action = new Action(
FlightSqlUtils.FLIGHT_SQL_CLOSEPREPAREDSTATEMENT.getType(),
FlightSqlUtils.FLIGHT_SQL_CLOSE_PREPARED_STATEMENT.getType(),
Any.pack(ActionClosePreparedStatementRequest.newBuilder()
.setPreparedStatementHandle(preparedStatementResult.getPreparedStatementHandle())
.build())
Expand Down
42 changes: 21 additions & 21 deletions java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,11 @@ default void listActions(CallContext context, StreamListener<ActionType> listene
@Override
default void doAction(CallContext context, Action action, StreamListener<Result> listener) {
final String actionType = action.getType();
if (actionType.equals(FlightSqlUtils.FLIGHT_SQL_CREATEPREPAREDSTATEMENT.getType())) {
if (actionType.equals(FlightSqlUtils.FLIGHT_SQL_CREATE_PREPARED_STATEMENT.getType())) {
final ActionCreatePreparedStatementRequest request = FlightSqlUtils.unpackAndParseOrThrow(action.getBody(),
ActionCreatePreparedStatementRequest.class);
createPreparedStatement(request, context, listener);
} else if (actionType.equals(FlightSqlUtils.FLIGHT_SQL_CLOSEPREPAREDSTATEMENT.getType())) {
} else if (actionType.equals(FlightSqlUtils.FLIGHT_SQL_CLOSE_PREPARED_STATEMENT.getType())) {
final ActionClosePreparedStatementRequest request = FlightSqlUtils.unpackAndParseOrThrow(action.getBody(),
ActionClosePreparedStatementRequest.class);
closePreparedStatement(request, context, listener);
Expand Down Expand Up @@ -566,38 +566,38 @@ final class Schemas {
public static final Schema GET_TABLES_SCHEMA = new Schema(Arrays.asList(
Field.nullable("catalog_name", MinorType.VARCHAR.getType()),
Field.nullable("schema_name", MinorType.VARCHAR.getType()),
Field.nullable("table_name", MinorType.VARCHAR.getType()),
Field.nullable("table_type", MinorType.VARCHAR.getType()),
Field.nullable("table_schema", MinorType.VARBINARY.getType())));
Field.notNullable("table_name", MinorType.VARCHAR.getType()),
Field.notNullable("table_type", MinorType.VARCHAR.getType()),
Field.notNullable("table_schema", MinorType.VARBINARY.getType())));
public static final Schema GET_TABLES_SCHEMA_NO_SCHEMA = new Schema(Arrays.asList(
Field.nullable("catalog_name", MinorType.VARCHAR.getType()),
Field.nullable("schema_name", MinorType.VARCHAR.getType()),
Field.nullable("table_name", MinorType.VARCHAR.getType()),
Field.nullable("table_type", MinorType.VARCHAR.getType())));
Field.notNullable("table_name", MinorType.VARCHAR.getType()),
Field.notNullable("table_type", MinorType.VARCHAR.getType())));
public static final Schema GET_CATALOGS_SCHEMA = new Schema(
Collections.singletonList(new Field("catalog_name", FieldType.nullable(MinorType.VARCHAR.getType()), null)));
Collections.singletonList(Field.notNullable("catalog_name", MinorType.VARCHAR.getType())));
public static final Schema GET_TABLE_TYPES_SCHEMA =
new Schema(Collections.singletonList(Field.nullable("table_type", MinorType.VARCHAR.getType())));
new Schema(Collections.singletonList(Field.notNullable("table_type", MinorType.VARCHAR.getType())));
public static final Schema GET_SCHEMAS_SCHEMA = new Schema(
Arrays.asList(Field.nullable("catalog_name", MinorType.VARCHAR.getType()),
Field.nullable("schema_name", MinorType.VARCHAR.getType())));
Field.notNullable("schema_name", MinorType.VARCHAR.getType())));
public static final Schema GET_IMPORTED_AND_EXPORTED_KEYS_SCHEMA = new Schema(Arrays.asList(
Field.nullable("pk_catalog_name", MinorType.VARCHAR.getType()),
Field.nullable("pk_schema_name", MinorType.VARCHAR.getType()),
Field.nullable("pk_table_name", MinorType.VARCHAR.getType()),
Field.nullable("pk_column_name", MinorType.VARCHAR.getType()),
Field.notNullable("pk_table_name", MinorType.VARCHAR.getType()),
Field.notNullable("pk_column_name", MinorType.VARCHAR.getType()),
Field.nullable("fk_catalog_name", MinorType.VARCHAR.getType()),
Field.nullable("fk_schema_name", MinorType.VARCHAR.getType()),
Field.nullable("fk_table_name", MinorType.VARCHAR.getType()),
Field.nullable("fk_column_name", MinorType.VARCHAR.getType()),
Field.nullable("key_sequence", MinorType.INT.getType()),
Field.notNullable("fk_table_name", MinorType.VARCHAR.getType()),
Field.notNullable("fk_column_name", MinorType.VARCHAR.getType()),
Field.notNullable("key_sequence", MinorType.INT.getType()),
Field.nullable("fk_key_name", MinorType.VARCHAR.getType()),
Field.nullable("pk_key_name", MinorType.VARCHAR.getType()),
Field.nullable("update_rule", new ArrowType.Int(8, false)),
Field.nullable("delete_rule", new ArrowType.Int(8, false))));
Field.notNullable("update_rule", new ArrowType.Int(8, false)),
Field.notNullable("delete_rule", new ArrowType.Int(8, false))));
public static final Schema GET_SQL_INFO_SCHEMA =
new Schema(Arrays.asList(
Field.nullable("info_name", new ArrowType.Int(32, false)),
Field.notNullable("info_name", new ArrowType.Int(32, false)),
new Field("value",
// dense_union<string_value: string, int_value: int32, bigint_value: int64, int32_bitmask: int32>
new FieldType(true, new Union(UnionMode.Dense, new int[] {0, 1, 2, 3}), /*dictionary=*/null),
Expand All @@ -609,9 +609,9 @@ final class Schemas {
public static final Schema GET_PRIMARY_KEYS_SCHEMA = new Schema(Arrays.asList(
Field.nullable("catalog_name", MinorType.VARCHAR.getType()),
Field.nullable("schema_name", MinorType.VARCHAR.getType()),
Field.nullable("table_name", MinorType.VARCHAR.getType()),
Field.nullable("column_name", MinorType.VARCHAR.getType()),
Field.nullable("key_sequence", MinorType.INT.getType()),
Field.notNullable("table_name", MinorType.VARCHAR.getType()),
Field.notNullable("column_name", MinorType.VARCHAR.getType()),
Field.notNullable("key_sequence", MinorType.INT.getType()),
Field.nullable("key_name", MinorType.VARCHAR.getType())));

private Schemas() {
Expand Down
8 changes: 4 additions & 4 deletions java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@
* Utilities to work with Flight SQL semantics.
*/
public final class FlightSqlUtils {
public static final ActionType FLIGHT_SQL_CREATEPREPAREDSTATEMENT = new ActionType("CreatePreparedStatement",
public static final ActionType FLIGHT_SQL_CREATE_PREPARED_STATEMENT = new ActionType("CreatePreparedStatement",
"Creates a reusable prepared statement resource on the server. \n" +
"Request Message: ActionCreatePreparedStatementRequest\n" +
"Response Message: ActionCreatePreparedStatementResult");

public static final ActionType FLIGHT_SQL_CLOSEPREPAREDSTATEMENT = new ActionType("ClosePreparedStatement",
public static final ActionType FLIGHT_SQL_CLOSE_PREPARED_STATEMENT = new ActionType("ClosePreparedStatement",
"Closes a reusable prepared statement resource on the server. \n" +
"Request Message: ActionClosePreparedStatementRequest\n" +
"Response Message: N/A");

public static final List<ActionType> FLIGHT_SQL_ACTIONS = ImmutableList.of(
FLIGHT_SQL_CREATEPREPAREDSTATEMENT,
FLIGHT_SQL_CLOSEPREPAREDSTATEMENT
FLIGHT_SQL_CREATE_PREPARED_STATEMENT,
FLIGHT_SQL_CLOSE_PREPARED_STATEMENT
);

/**
Expand Down
Loading

0 comments on commit 26482f6

Please sign in to comment.