Skip to content

GH-46087: [FlightSQL] Allow returning column remarks in FlightSQL's CommandGetTables #46110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidavidm merged 6 commits into from
May 3, 2025
Merged

GH-46087: [FlightSQL] Allow returning column remarks in FlightSQL's CommandGetTables #46110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1ce18e6
GH-46087: [FlightSQL] Allow returning column remarks in FlightSQL's C…
mateuszrzeszutek Apr 11, 2025
48d01a4
revert integration test changes
mateuszrzeszutek Apr 11, 2025
27fe039
fix the typo in FlightSql.proto
mateuszrzeszutek Apr 23, 2025
9d95fc6
Add extra note about the recently added metadata field
mateuszrzeszutek Apr 25, 2025
40fc2f7
restore integration test changes
mateuszrzeszutek Apr 27, 2025
448bb5a
fix a typo
mateuszrzeszutek Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/integration_tests/test_integration.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,7 @@ const std::shared_ptr<Schema>& GetQuerySchema() {
.IsSearchable(true)
.CatalogName("catalog_test")
.Precision(100)
.Remarks("test column")
.Build()
.metadata_map())});
return kSchema;
Expand All @@ -1187,6 +1188,7 @@ std::shared_ptr<Schema> GetQueryWithTransactionSchema() {
.IsSearchable(true)
.CatalogName("catalog_test")
.Precision(100)
.Remarks("test column")
.Build()
.metadata_map())});
return kSchema;
Expand Down
11 changes: 11 additions & 0 deletions cpp/src/arrow/flight/sql/column_metadata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const char* ColumnMetadata::kIsAutoIncrement = "ARROW:FLIGHT:SQL:IS_AUTO_INCREME
const char* ColumnMetadata::kIsCaseSensitive = "ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE";
const char* ColumnMetadata::kIsReadOnly = "ARROW:FLIGHT:SQL:IS_READ_ONLY";
const char* ColumnMetadata::kIsSearchable = "ARROW:FLIGHT:SQL:IS_SEARCHABLE";
const char* ColumnMetadata::kRemarks = "ARROW:FLIGHT:SQL:REMARKS";

ColumnMetadata::ColumnMetadata(
std::shared_ptr<const arrow::KeyValueMetadata> metadata_map)
Expand Down Expand Up @@ -114,6 +115,10 @@ arrow::Result<bool> ColumnMetadata::GetIsSearchable() const {
return StringToBoolean(is_case_sensitive);
}

arrow::Result<std::string> ColumnMetadata::GetRemarks() const {
return metadata_map_->Get(kRemarks);
}

ColumnMetadata::ColumnMetadataBuilder ColumnMetadata::Builder() {
return ColumnMetadataBuilder{};
}
Expand Down Expand Up @@ -185,6 +190,12 @@ ColumnMetadata::ColumnMetadataBuilder::IsSearchable(bool is_searchable) {
return *this;
}

ColumnMetadata::ColumnMetadataBuilder& ColumnMetadata::ColumnMetadataBuilder::Remarks(
const std::string& remarks) {
metadata_map_->Append(ColumnMetadata::kRemarks, remarks);
return *this;
}

ColumnMetadata::ColumnMetadataBuilder::ColumnMetadataBuilder()
: metadata_map_(std::make_shared<arrow::KeyValueMetadata>()) {}

Expand Down
12 changes: 12 additions & 0 deletions cpp/src/arrow/flight/sql/column_metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata {
/// \brief Constant variable to hold the value of the key that
/// will be used in the KeyValueMetadata class.
static const char* kIsSearchable;
/// \brief Constant variable to hold the value of the key that
/// will be used in the KeyValueMetadata class.
static const char* kRemarks;

/// \brief Static initializer.
static ColumnMetadataBuilder Builder();
Expand Down Expand Up @@ -110,6 +113,10 @@ class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata {
/// \return The IsSearchable.
arrow::Result<bool> GetIsSearchable() const;

/// \brief Return the Remarks set in the KeyValueMetadata.
/// \return The Remarks.
arrow::Result<std::string> GetRemarks() const;

/// \brief Return the KeyValueMetadata.
/// \return The KeyValueMetadata.
const std::shared_ptr<const arrow::KeyValueMetadata>& metadata_map() const;
Expand Down Expand Up @@ -169,6 +176,11 @@ class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata {
/// \return A ColumnMetadataBuilder.
ColumnMetadataBuilder& IsSearchable(bool is_searchable);

/// \brief Set the column description in the KeyValueMetadata object.
/// \param[in] remarks The comment describing column.
/// \return A ColumnMetadataBuilder.
ColumnMetadataBuilder& Remarks(const std::string& remarks);

ColumnMetadata Build() const;

private:
Expand Down
6 changes: 5 additions & 1 deletion format/FlightSql.proto
View file
Open in desktop
Copy link
Member

@lidavidm lidavidm Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that I was thinking about, though, should we note that this field was added after the others and clients should be prepared to find it missing?

Copy link
Contributor Author

@mateuszrzeszutek mateuszrzeszutek Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,7 @@ message CommandGetDbSchemas {
* - ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE - "1" indicates if the column is case-sensitive, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_READ_ONLY - "1" indicates if the column is read only, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_SEARCHABLE - "1" indicates if the column is searchable via WHERE clause, "0" otherwise.
* - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. This field has been added after all others, clients should be prepared to find it missing.
* The returned data should be ordered by catalog_name, db_schema_name, table_name, then table_type, followed by table_schema if requested.
*/
message CommandGetTables {
Expand Down Expand Up @@ -1678,6 +1679,7 @@ message ActionEndSavepointRequest {
* - ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE - "1" indicates if the column is case-sensitive, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_READ_ONLY - "1" indicates if the column is read only, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_SEARCHABLE - "1" indicates if the column is searchable via WHERE clause, "0" otherwise.
* - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. This field has been added after all others, clients should be prepared to find it missing.
* - GetFlightInfo: execute the query.
*/
message CommandStatementQuery {
Expand All @@ -1703,6 +1705,7 @@ message CommandStatementQuery {
* - ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE - "1" indicates if the column is case-sensitive, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_READ_ONLY - "1" indicates if the column is read only, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_SEARCHABLE - "1" indicates if the column is searchable via WHERE clause, "0" otherwise.
* - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. This field has been added after all others, clients should be prepared to find it missing.
* - GetFlightInfo: execute the query.
* - DoPut: execute the query.
*/
Expand Down Expand Up @@ -1739,6 +1742,7 @@ message TicketStatementQuery {
* - ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE - "1" indicates if the column is case-sensitive, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_READ_ONLY - "1" indicates if the column is read only, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_SEARCHABLE - "1" indicates if the column is searchable via WHERE clause, "0" otherwise.
* - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. This field has been added after all others, clients should be prepared to find it missing.
*
* If the schema is retrieved after parameter values have been bound with DoPut, then the server should account
* for the parameters when determining the schema.
Expand Down Expand Up @@ -1857,7 +1861,7 @@ message DoPutPreparedStatementResult {
// statement should be considered invalid, and all subsequent requests for this prepared
// statement must use this new handle.
// The updated handle allows implementing query parameters with stateless services.
//
//
// When an updated handle is not provided by the server, clients should contiue
// using the previous handle provided by `ActionCreatePreparedStatementResonse`.
optional bytes prepared_statement_handle = 1;
Expand Down
Loading