From 1ce18e63082326240a0d9b56d1ac043021ed5891 Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Fri, 11 Apr 2025 10:48:35 +0200 Subject: [PATCH 1/6] GH-46087: [FlightSQL] Allow returning column remarks in FlightSQL's CommandGetTables --- .../flight/integration_tests/test_integration.cc | 2 ++ cpp/src/arrow/flight/sql/column_metadata.cc | 11 +++++++++++ cpp/src/arrow/flight/sql/column_metadata.h | 12 ++++++++++++ format/FlightSql.proto | 6 +++++- 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/flight/integration_tests/test_integration.cc b/cpp/src/arrow/flight/integration_tests/test_integration.cc index f38076822c7..25600b92cd4 100644 --- a/cpp/src/arrow/flight/integration_tests/test_integration.cc +++ b/cpp/src/arrow/flight/integration_tests/test_integration.cc @@ -1167,6 +1167,7 @@ const std::shared_ptr& GetQuerySchema() { .IsSearchable(true) .CatalogName("catalog_test") .Precision(100) + .Remarks("test column") .Build() .metadata_map())}); return kSchema; @@ -1187,6 +1188,7 @@ std::shared_ptr GetQueryWithTransactionSchema() { .IsSearchable(true) .CatalogName("catalog_test") .Precision(100) + .Remarks("test column") .Build() .metadata_map())}); return kSchema; diff --git a/cpp/src/arrow/flight/sql/column_metadata.cc b/cpp/src/arrow/flight/sql/column_metadata.cc index c855e2f09af..e9b9db05d86 100644 --- a/cpp/src/arrow/flight/sql/column_metadata.cc +++ b/cpp/src/arrow/flight/sql/column_metadata.cc @@ -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 metadata_map) @@ -114,6 +115,10 @@ arrow::Result ColumnMetadata::GetIsSearchable() const { return StringToBoolean(is_case_sensitive); } +arrow::Result ColumnMetadata::GetRemarks() const { + return metadata_map_->Get(kRemarks); +} + ColumnMetadata::ColumnMetadataBuilder ColumnMetadata::Builder() { return ColumnMetadataBuilder{}; } @@ -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()) {} diff --git a/cpp/src/arrow/flight/sql/column_metadata.h b/cpp/src/arrow/flight/sql/column_metadata.h index 0eb53f3e0bb..96a8e6493bd 100644 --- a/cpp/src/arrow/flight/sql/column_metadata.h +++ b/cpp/src/arrow/flight/sql/column_metadata.h @@ -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(); @@ -110,6 +113,10 @@ class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata { /// \return The IsSearchable. arrow::Result GetIsSearchable() const; + /// \brief Return the Remarks set in the KeyValueMetadata. + /// \return The IsSearchable. + arrow::Result GetRemarks() const; + /// \brief Return the KeyValueMetadata. /// \return The KeyValueMetadata. const std::shared_ptr& metadata_map() const; @@ -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 decripting column. + /// \return A ColumnMetadataBuilder. + ColumnMetadataBuilder& Remarks(const std::string& remarks); + ColumnMetadata Build() const; private: diff --git a/format/FlightSql.proto b/format/FlightSql.proto index ef1ae7513d4..4eb8ea40526 100644 --- a/format/FlightSql.proto +++ b/format/FlightSql.proto @@ -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 descripting column. * 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 { @@ -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 descripting column. * - GetFlightInfo: execute the query. */ message CommandStatementQuery { @@ -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 descripting column. * - GetFlightInfo: execute the query. * - DoPut: execute the query. */ @@ -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 descripting column. * * 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. @@ -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; From 48d01a44fff23bdd4186f2e299f9ceb3a30cb11b Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Fri, 11 Apr 2025 12:34:29 +0200 Subject: [PATCH 2/6] revert integration test changes --- cpp/src/arrow/flight/integration_tests/test_integration.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/cpp/src/arrow/flight/integration_tests/test_integration.cc b/cpp/src/arrow/flight/integration_tests/test_integration.cc index 25600b92cd4..f38076822c7 100644 --- a/cpp/src/arrow/flight/integration_tests/test_integration.cc +++ b/cpp/src/arrow/flight/integration_tests/test_integration.cc @@ -1167,7 +1167,6 @@ const std::shared_ptr& GetQuerySchema() { .IsSearchable(true) .CatalogName("catalog_test") .Precision(100) - .Remarks("test column") .Build() .metadata_map())}); return kSchema; @@ -1188,7 +1187,6 @@ std::shared_ptr GetQueryWithTransactionSchema() { .IsSearchable(true) .CatalogName("catalog_test") .Precision(100) - .Remarks("test column") .Build() .metadata_map())}); return kSchema; From 27fe0398ae012260bd18781da26de7b59e2966bb Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Wed, 23 Apr 2025 08:48:20 +0200 Subject: [PATCH 3/6] fix the typo in FlightSql.proto --- format/FlightSql.proto | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/format/FlightSql.proto b/format/FlightSql.proto index 4eb8ea40526..566230c2a6b 100644 --- a/format/FlightSql.proto +++ b/format/FlightSql.proto @@ -1212,7 +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 descripting column. + * - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. * 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 { @@ -1679,7 +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 descripting column. + * - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. * - GetFlightInfo: execute the query. */ message CommandStatementQuery { @@ -1705,7 +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 descripting column. + * - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. * - GetFlightInfo: execute the query. * - DoPut: execute the query. */ @@ -1742,7 +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 descripting column. + * - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column. * * 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. From 9d95fc6fe0285e4d2b6d5f2b3b73376d82da8623 Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Fri, 25 Apr 2025 12:36:17 +0200 Subject: [PATCH 4/6] Add extra note about the recently added metadata field --- format/FlightSql.proto | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/format/FlightSql.proto b/format/FlightSql.proto index 566230c2a6b..25dc1318743 100644 --- a/format/FlightSql.proto +++ b/format/FlightSql.proto @@ -1212,7 +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. + * - 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 { @@ -1679,7 +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. + * - 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 { @@ -1705,7 +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. + * - 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. */ @@ -1742,7 +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. + * - 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. From 40fc2f7e736169e330d6d202f738e8a2e5a9400b Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Sun, 27 Apr 2025 10:17:40 +0200 Subject: [PATCH 5/6] restore integration test changes This reverts commit cd3008b172d57ca562a4e0809b2d01cb12d36cdc. --- cpp/src/arrow/flight/integration_tests/test_integration.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/src/arrow/flight/integration_tests/test_integration.cc b/cpp/src/arrow/flight/integration_tests/test_integration.cc index f38076822c7..25600b92cd4 100644 --- a/cpp/src/arrow/flight/integration_tests/test_integration.cc +++ b/cpp/src/arrow/flight/integration_tests/test_integration.cc @@ -1167,6 +1167,7 @@ const std::shared_ptr& GetQuerySchema() { .IsSearchable(true) .CatalogName("catalog_test") .Precision(100) + .Remarks("test column") .Build() .metadata_map())}); return kSchema; @@ -1187,6 +1188,7 @@ std::shared_ptr GetQueryWithTransactionSchema() { .IsSearchable(true) .CatalogName("catalog_test") .Precision(100) + .Remarks("test column") .Build() .metadata_map())}); return kSchema; From 448bb5a749da86c9b05042c870c24c7f6338e28f Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Mon, 28 Apr 2025 09:42:04 +0200 Subject: [PATCH 6/6] fix a typo --- cpp/src/arrow/flight/sql/column_metadata.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/flight/sql/column_metadata.h b/cpp/src/arrow/flight/sql/column_metadata.h index 96a8e6493bd..fe29df90401 100644 --- a/cpp/src/arrow/flight/sql/column_metadata.h +++ b/cpp/src/arrow/flight/sql/column_metadata.h @@ -114,7 +114,7 @@ class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata { arrow::Result GetIsSearchable() const; /// \brief Return the Remarks set in the KeyValueMetadata. - /// \return The IsSearchable. + /// \return The Remarks. arrow::Result GetRemarks() const; /// \brief Return the KeyValueMetadata. @@ -177,7 +177,7 @@ class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata { ColumnMetadataBuilder& IsSearchable(bool is_searchable); /// \brief Set the column description in the KeyValueMetadata object. - /// \param[in] remarks The comment decripting column. + /// \param[in] remarks The comment describing column. /// \return A ColumnMetadataBuilder. ColumnMetadataBuilder& Remarks(const std::string& remarks);