Skip to content

GH-49552: [C++][FlightRPC][ODBC] Enable ODBC test build on Linux - #49668

Merged
kou merged 2 commits into
apache:mainfrom
Bit-Quill:gh-49552-linux-test-build
May 21, 2026
Merged

GH-49552: [C++][FlightRPC][ODBC] Enable ODBC test build on Linux#49668
kou merged 2 commits into
apache:mainfrom
Bit-Quill:gh-49552-linux-test-build

Conversation

@justing-bq

@justing-bqjusting-bq commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

The test suite needs to be updated so it will build on Linux in addition to Windows & Mac.
Resolves#49552

What changes are included in this PR?

Miscellaneous changes to get the tests building on Linux.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@justing-bqjusting-bq changed the title [C++][FlightRPC][ODBC] Enable ODBC test build on LinuxGH-49552: [C++][FlightRPC][ODBC] Enable ODBC test build on LinuxApr 6, 2026
@github-actions

Copy link
Copy Markdown

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

@justing-bq
justing-bqforce-pushed the gh-49552-linux-test-build branch from 71ade9f to ccf583aCompareApril 15, 2026 17:39
@justing-bq
justing-bq marked this pull request as ready for review April 15, 2026 17:40
@justing-bq

Copy link
Copy Markdown
ContributorAuthor

@lidavidm@kou
Please take a look when you have a chance.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enables building the Flight SQL ODBC C++ test suite on Linux by removing the platform gate in CMake and updating tests/helpers to better handle platform differences in SQLWCHAR and wide-string handling.

Changes:

  • Always add the ODBC tests subdirectory when ARROW_BUILD_TESTS is enabled (including Linux).
  • Refactor many tests to avoid relying on L""/wcslen assumptions and to use helper macros/utilities for SQLWCHAR.
  • Adjust CI to exclude running the ODBC test binary on Linux while still building it.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
cpp/src/arrow/flight/sql/odbc/tests/type_info_test.ccSwitches expected column name handling and replaces NULL sentinel usage with numeric values.
cpp/src/arrow/flight/sql/odbc/tests/tables_test.ccUpdates SQLWCHAR parameter initialization and column-name comparisons for portability.
cpp/src/arrow/flight/sql/odbc/tests/statement_test.ccIntroduces ASSIGN_SQLWCHAR_ARR_AND_LEN usage and adjusts string comparisons/conversions.
cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.ccUses the new SQLWCHAR assignment helper macro for query strings.
cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.hAdds Linux-specific SQLWCHAR assignment macros and declares new conversion helpers.
cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.ccImplements new SQLWCHAR length/conversion helpers and tweaks DSN registration call.
cpp/src/arrow/flight/sql/odbc/tests/errors_test.ccAvoids constructing std::wstring directly from SQLWCHAR* by converting to UTF-8 first.
cpp/src/arrow/flight/sql/odbc/tests/connection_test.ccMakes output buffers consistently zero-initialized.
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.ccRenames helper and converts many wide-string assertions to use ConvertToWString.
cpp/src/arrow/flight/sql/odbc/tests/columns_test.ccUpdates SQLWCHAR parameter handling and metadata assertions for Linux builds.
cpp/src/arrow/flight/sql/odbc/CMakeLists.txtRemoves Windows/macOS-only gating so tests are built on Linux too.
ci/scripts/cpp_test.shExcludes arrow-flight-sql-odbc-test from Linux test execution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc
Comment threadcpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc
Comment threadcpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated
Comment threadcpp/src/arrow/flight/sql/odbc/tests/statement_test.cc
Comment threadcpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h Outdated
Comment threadcpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc
Comment threadcpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc Outdated

@alinaliBQalinaliBQ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have 1 comment

Comment threadcpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc Outdated

@alinaliBQalinaliBQ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h
Comment threadcpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc
@justing-bq
justing-bqforce-pushed the gh-49552-linux-test-build branch from 27c116c to df15956CompareApril 22, 2026 23:19
@justing-bq

Copy link
Copy Markdown
ContributorAuthor

@lidavidm@kou
Please take a look when you have time.

@justing-bq
justing-bqforce-pushed the gh-49552-linux-test-build branch from df15956 to 1a69faeCompareApril 27, 2026 21:15
@justing-bq
justing-bqforce-pushed the gh-49552-linux-test-build branch 3 times, most recently from 240837a to e34824aCompareMay 5, 2026 21:24
@justing-bq
justing-bqforce-pushed the gh-49552-linux-test-build branch from e34824a to 6d036ceCompareMay 19, 2026 18:46
@justing-bq

Copy link
Copy Markdown
ContributorAuthor

@lidavidm@kou
Please take a look when you have time.

kou
kou approved these changes May 20, 2026

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

The ODBC test on Linux is failing. It's out-of-scope of this PR, right?

fi
case "$(uname)" in
Linux)
exclude_tests+=("arrow-flight-sql-odbc-test")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We remove this later, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Correct. This gets removed in 9e8272d.

@koukouMay 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Which PR does include the commit?
GH-49786?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

#49786 which will be undrafted as soon as this PR gets merged.

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels May 20, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comment threadci/scripts/cpp_test.sh
@justing-bq
justing-bqforce-pushed the gh-49552-linux-test-build branch from 6d036ce to 682bab2CompareMay 20, 2026 17:46
@justing-bq

Copy link
Copy Markdown
ContributorAuthor

@kou@lidavidm
Comments addressed. Please review.

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels May 21, 2026
@kou
kou merged commit 62cdda9 into apache:mainMay 21, 2026
61 of 66 checks passed
@koukou removed the awaiting changes Awaiting changes label May 21, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 62cdda9.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
apache#49668)
### Rationale for this change
The test suite needs to be updated so it will build on Linux in addition to Windows & Mac.
Resolvesapache#49552
### What changes are included in this PR?
Miscellaneous changes to get the tests building on Linux.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
* GitHub Issue: apache#49552
Authored-by: justing-bq <62349012+justing-bq@users.noreply.github.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
lidavidm pushed a commit that referenced this pull request Jun 12, 2026
)
### Rationale for this change
After getting the tests building on Linux, the next step is to get all the tests passing.
Resolves#49785.
### What changes are included in this PR?
Miscellaneous fixes.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
Depends on #49668 and #49784 before being ready for review.
* GitHub Issue: #49785
Authored-by: justing-bq <62349012+justing-bq@users.noreply.github.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for freeto 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.

[C++][FlightRPC][ODBC] Enable ODBC test build on Linux

4 participants

@justing-bq@kou@alinaliBQ