Skip to content

Conversation

@alinaliBQ
Copy link
Contributor

@alinaliBQ alinaliBQ commented Dec 4, 2025

Rationale for this change

Replace boost::optional with std::optional to resolve build failures on MSVC Windows CI. This PR is extracted from #48313.

What changes are included in this PR?

Replace boost::optional with std::optional in ODBC.

Are these changes tested?

Yes, locally on MSVC

Are there any user-facing changes?

N/A

@alinaliBQ alinaliBQ requested a review from lidavidm as a code owner December 4, 2025 00:25
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

⚠️ GitHub issue #48084 has been automatically assigned in GitHub to PR creator.

Comment on lines 75 to 76
return std::optional<std::pair<arrow::Result<FlightStreamChunk>,
std::shared_ptr<FlightSqlClient>>>{};
Copy link
Member

Choose a reason for hiding this comment

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

nit: does nullopt not work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes nullopt works, fixed


struct MetadataSettings {
boost::optional<int32_t> string_column_length{boost::none};
std::optional<int32_t> string_column_length{std::nullopt};
Copy link
Member

Choose a reason for hiding this comment

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

The default initialization is nullopt so there's no need to write it out explicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, good catch, fixed

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Dec 4, 2025
Use `std::nullopt;` and specify return type.
Copy link
Contributor Author

@alinaliBQ alinaliBQ left a comment

Choose a reason for hiding this comment

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

@lidavidm Thanks, addressed all comments

Comment on lines 75 to 76
return std::optional<std::pair<arrow::Result<FlightStreamChunk>,
std::shared_ptr<FlightSqlClient>>>{};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes nullopt works, fixed


struct MetadataSettings {
boost::optional<int32_t> string_column_length{boost::none};
std::optional<int32_t> string_column_length{std::nullopt};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, good catch, fixed

@lidavidm lidavidm merged commit 90261d6 into apache:main Dec 4, 2025
47 checks passed
@lidavidm lidavidm removed the awaiting merge Awaiting merge label Dec 4, 2025
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 90261d6.

There was 1 benchmark result indicating a performance regression:

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants