Skip to content

Fix segfault issue from empty metadata - #81

Merged
alinaliBQ merged 3 commits into
apache-odbcfrom
check-metadata-field
Aug 6, 2025
Merged

Fix segfault issue from empty metadata#81
alinaliBQ merged 3 commits into
apache-odbcfrom
check-metadata-field

Conversation

@alinaliBQ

@alinaliBQ alinaliBQ commented Aug 5, 2025

Copy link
Copy Markdown

Transform_inner was having a seg fault error because metadata() return result was not being checked for null value. I see in flight_sql/flight_sql_result_set_metadata.cc there is already a check for null metadata(), so I re-used this check in Transform_inner.

std::shared_ptr<const arrow::KeyValueMetadata> empty_metadata_map(
new arrow::KeyValueMetadata);
inline arrow::flight::sql::ColumnMetadata GetMetadata(
const std::shared_ptr<Field>& field) {
const auto& metadata_map = field->metadata();
arrow::flight::sql::ColumnMetadata metadata(metadata_map ? metadata_map
: empty_metadata_map);

  • Also fixed: Use metadata.GetTypeName() from flightsql-odbc

@alinaliBQ
alinaliBQ marked this pull request as ready for review August 5, 2025 21:24
const auto& metadata_map = field->metadata();
std::shared_ptr<const arrow::KeyValueMetadata> empty_metadata_map(
new arrow::KeyValueMetadata);
ColumnMetadata metadata(metadata_map ? metadata_map : empty_metadata_map);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's change the ColumnMetadata object itself to handle NULL safely and internally create an empty owned map (since it's easy for this to occur elsewhere)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yea that makes sense, I have updated the code to change ColumnMetadata.

There is an API Field::HasMetadata in arrow that checks if metadata is null, but seems it's not as commonly used.

arrow/cpp/src/arrow/type.cc

Lines 304 to 306 in a6bc579

bool Field::HasMetadata() const {
return (metadata_ != nullptr) && (metadata_->size() > 0);
}

@alinaliBQ
alinaliBQ requested a review from jduo August 5, 2025 22:38
@alinaliBQ
alinaliBQ merged commit 1012ee0 into apache-odbc Aug 6, 2025
24 checks passed
@alinaliBQ
alinaliBQ deleted the check-metadata-field branch August 6, 2025 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants