From de1db398e17efef1b161f4fde37127d1eb80be17 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 1 Aug 2025 15:45:10 -0700 Subject: [PATCH 1/3] Fix segfault issue from empty metadata Use empty map in bug fix --- .../odbc/flight_sql/flight_sql_statement_get_columns.cc | 9 ++++++--- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc index d3250401193d..d00e7712deae 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc @@ -115,7 +115,10 @@ Result> Transform_inner( odbcabstraction::SqlDataType data_type_v3 = GetDataTypeFromArrowField_V3(field, metadata_settings.use_wide_char_); - ColumnMetadata metadata(field->metadata()); + const auto& metadata_map = field->metadata(); + std::shared_ptr empty_metadata_map( + new arrow::KeyValueMetadata); + ColumnMetadata metadata(metadata_map ? metadata_map : empty_metadata_map); data.table_cat = table_catalog; data.table_schem = table_schema; @@ -125,8 +128,8 @@ Result> Transform_inner( ? data_type_v3 : ConvertSqlDataTypeFromV3ToV2(data_type_v3); - // TODO: Use `metadata.GetTypeName()` when ARROW-16064 is merged. - const auto& type_name_result = field->metadata()->Get("ARROW:FLIGHT:SQL:TYPE_NAME"); + const auto& type_name_result = metadata.GetTypeName(); + data.type_name = type_name_result.ok() ? type_name_result.ValueOrDie() : GetTypeNameFromSqlDataType(data_type_v3); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 0331306e31d8..461e17fae5fb 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -962,7 +962,6 @@ SQLRETURN SQLFetch(SQLHSTMT stmt) { using ODBC::ODBCDescriptor; using ODBC::ODBCStatement; - return ODBCStatement::ExecuteWithDiagnostics(stmt, SQL_ERROR, [=]() { ODBCStatement* statement = reinterpret_cast(stmt); @@ -970,6 +969,7 @@ SQLRETURN SQLFetch(SQLHSTMT stmt) { // rowset. ODBCDescriptor* ard = statement->GetARD(); size_t rows = static_cast(ard->GetArraySize()); + if (statement->Fetch(rows)) { return SQL_SUCCESS; } else { From 2966e21b1527fa85418f251595013ade0d9205ae Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 5 Aug 2025 11:56:42 -0700 Subject: [PATCH 2/3] chore: trigger CI From 782ac8845dd0762a2eceb41963d867da14c51e7b Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 5 Aug 2025 15:06:49 -0700 Subject: [PATCH 3/3] Address comments from James --- cpp/src/arrow/flight/sql/column_metadata.cc | 11 +++++++++-- .../odbc/flight_sql/flight_sql_result_set_metadata.cc | 8 +------- .../flight_sql/flight_sql_statement_get_columns.cc | 5 +---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/cpp/src/arrow/flight/sql/column_metadata.cc b/cpp/src/arrow/flight/sql/column_metadata.cc index 30f557084b2d..8d2d2b4ddcac 100644 --- a/cpp/src/arrow/flight/sql/column_metadata.cc +++ b/cpp/src/arrow/flight/sql/column_metadata.cc @@ -58,8 +58,15 @@ const char* ColumnMetadata::kIsSearchable = "ARROW:FLIGHT:SQL:IS_SEARCHABLE"; const char* ColumnMetadata::kRemarks = "ARROW:FLIGHT:SQL:REMARKS"; ColumnMetadata::ColumnMetadata( - std::shared_ptr metadata_map) - : metadata_map_(std::move(metadata_map)) {} + std::shared_ptr metadata_map) { + if (metadata_map) { + metadata_map_ = std::move(metadata_map); + } else { + std::shared_ptr empty_metadata_map( + new arrow::KeyValueMetadata); + metadata_map_ = std::move(empty_metadata_map); + } +} arrow::Result ColumnMetadata::GetCatalogName() const { return metadata_map_->Get(kCatalogName); diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_metadata.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_metadata.cc index 710f7608ecfc..0fa6b03c4a7e 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_metadata.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_metadata.cc @@ -42,15 +42,9 @@ constexpr int32_t DefaultDecimalPrecision = 38; constexpr int32_t DefaultLengthForVariableLengthColumns = 1024; namespace { -std::shared_ptr empty_metadata_map( - new arrow::KeyValueMetadata); - inline arrow::flight::sql::ColumnMetadata GetMetadata( const std::shared_ptr& field) { - const auto& metadata_map = field->metadata(); - - arrow::flight::sql::ColumnMetadata metadata(metadata_map ? metadata_map - : empty_metadata_map); + arrow::flight::sql::ColumnMetadata metadata(field->metadata()); return metadata; } diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc index d00e7712deae..f61c198cd23f 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.cc @@ -115,10 +115,7 @@ Result> Transform_inner( odbcabstraction::SqlDataType data_type_v3 = GetDataTypeFromArrowField_V3(field, metadata_settings.use_wide_char_); - const auto& metadata_map = field->metadata(); - std::shared_ptr empty_metadata_map( - new arrow::KeyValueMetadata); - ColumnMetadata metadata(metadata_map ? metadata_map : empty_metadata_map); + ColumnMetadata metadata(field->metadata()); data.table_cat = table_catalog; data.table_schem = table_schema;