Skip to content

GH-49651: [C++][FlightRPC] Fix ODBC Linux test segmentation fault#49688

Open
alinaliBQ wants to merge 3 commits intoapache:mainfrom
Bit-Quill:gh-49651-linux-segfault
Open

GH-49651: [C++][FlightRPC] Fix ODBC Linux test segmentation fault#49688
alinaliBQ wants to merge 3 commits intoapache:mainfrom
Bit-Quill:gh-49651-linux-segfault

Conversation

@alinaliBQ
Copy link
Copy Markdown
Collaborator

@alinaliBQ alinaliBQ commented Apr 7, 2026

Rationale for this change

GH-49651

What changes are included in this PR?

  • On Linux, change to use static test linking for ODBC only

Note: enabling Linux test build will be in a separate PR

Are these changes tested?

Tested on local machine and in our local repository

Are there any user-facing changes?

N/A

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

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

@alinaliBQ alinaliBQ marked this pull request as ready for review April 7, 2026 22:44
@alinaliBQ
Copy link
Copy Markdown
Collaborator Author

@lidavidm @kou This PR is ready for review, please have a look

@lidavidm
Copy link
Copy Markdown
Member

lidavidm commented Apr 8, 2026

Hmm, why are there two copies of gRPC in the first place?

@alinaliBQ
Copy link
Copy Markdown
Collaborator Author

Hmm, why are there two copies of gRPC in the first place?

My understanding is, the arrow_flight_sql_odbc_shared has a copy of gRPC, and the tests have a copy of gRPC because it uses gRPC to set up the mock server. So when we dynamically link the tests to the shared ODBC lib, we end up with 2 copies.

@lidavidm
Copy link
Copy Markdown
Member

lidavidm commented Apr 9, 2026

So then arrow_flight_sql_odbc_shared (or the tests) is currently always statically linking gRPC? Because if they both dynamically link gRPC, there should be no problem.

@alinaliBQ
Copy link
Copy Markdown
Collaborator Author

So then arrow_flight_sql_odbc_shared (or the tests) is currently always statically linking gRPC? Because if they both dynamically link gRPC, there should be no problem.

Yes, arrow_flight_sql_odbc_shared is statically linked with gRPC on macOS from #49219. This enables the ODBC .dylib to act as a self-contained library without needing additional third party dependencies. And due to macOS Excel's sandboxed environment, ODBC .dylib files are usually statically linked to dependencies in order to be successfully loaded.

@lidavidm
Copy link
Copy Markdown
Member

I think it would be preferable to only link the ODBC tests statically instead of forcing it for all tests then. Or only statically link gRPC for "release" builds of the driver instead of during test (but that may not be possible)

@alinaliBQ alinaliBQ force-pushed the gh-49651-linux-segfault branch from f8302cd to 5eb872f Compare April 14, 2026 16:37
@alinaliBQ
Copy link
Copy Markdown
Collaborator Author

That makes sense. I have changed the ODBC tests to link statically on Linux instead of changing it for all tests in ubuntu-cpp-odbc service.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Apr 14, 2026
alinaliBQ and others added 2 commits April 15, 2026 14:50
* Resolve segfault issue on Linux and pass `ARROW_TEST_LINKAGE=static`

Pass `ARROW_TEST_LINKAGE=static`

* Clean up
@alinaliBQ alinaliBQ force-pushed the gh-49651-linux-segfault branch from 5eb872f to e456f1b Compare April 15, 2026 21:51
@lidavidm
Copy link
Copy Markdown
Member

I retried the failing pipelines. It seems C++ Extra / ODBC debug AMD64 macOS 15-intel times out; should we increase the runtime, or alternatively find a way to cut down on the build time? Maybe we don't need to build/run all Arrow tests for this config?

@lidavidm
Copy link
Copy Markdown
Member

Incidentally the build log notes lots of warnings that may be good to tackle.

/Users/runner/work/arrow/arrow/cpp/src/arrow/flight/sql/odbc/tests/columns_test.cc:367:10: warning: unused variable 'num_val' [-Wunused-variable]
  367 |   SQLLEN num_val = 0;
      |          ^~~~~~~
/Users/runner/work/arrow/arrow/cpp/src/arrow/flight/sql/odbc/tests/columns_test.cc:449:23: warning: implicit conversion of NULL constant to 'const SQLSMALLINT' (aka 'const short') [-Wnull-conversion]
  437 |   CheckMockSQLColumns(stmt,
      |   ~~~~~~~~~~~~~~~~~~~
  438 |                       std::wstring(L"main"),          // expected_catalog
  439 |                       std::wstring(L"foreignTable"),  // expected_table
  440 |                       std::wstring(L"id"),            // expected_column
  441 |                       SQL_BIGINT,                     // expected_data_type
  442 |                       std::wstring(L"BIGINT"),        // expected_type_name
  443 |                       10,  // expected_column_size (mock returns 10 instead of 19)
  444 |                       8,   // expected_buffer_length
  445 |                       15,  // expected_decimal_digits (mock returns 15 instead of 0)
  446 |                       10,  // expected_num_prec_radix
  447 |                       SQL_NULLABLE,           // expected_nullable
  448 |                       SQL_BIGINT,             // expected_sql_data_type
  449 |                       NULL,                   // expected_date_time_sub
      |                       ^~~~
      |                       0

@alinaliBQ
Copy link
Copy Markdown
Collaborator Author

I retried the failing pipelines. It seems C++ Extra / ODBC debug AMD64 macOS 15-intel times out; should we increase the runtime, or alternatively find a way to cut down on the build time? Maybe we don't need to build/run all Arrow tests for this config?

Yea I have noticed this recently as well, and I increased the runtime to 120 minutes. Currently, the CI runs tests only for the ODBC library and its dependencies. The extra build time might be due to the recent gRPC bump, as the newer version seems to take more time to compile.

@alinaliBQ
Copy link
Copy Markdown
Collaborator Author

Incidentally the build log notes lots of warnings that may be good to tackle.

👍 We are tackling these warnings in @justing-bq's PR https://github.com/apache/arrow/pull/49668/changes

@lidavidm
Copy link
Copy Markdown
Member

I retried the failing pipelines. It seems C++ Extra / ODBC debug AMD64 macOS 15-intel times out; should we increase the runtime, or alternatively find a way to cut down on the build time? Maybe we don't need to build/run all Arrow tests for this config?

Yea I have noticed this recently as well, and I increased the runtime to 120 minutes. Currently, the CI runs tests only for the ODBC library and its dependencies. The extra build time might be due to the recent gRPC bump, as the newer version seems to take more time to compile.

I think that's good reason to see if we can change that specific pipeline (and perhaps other ODBC pipelines) to only build ODBC (in a separate task)

I see C++ Extra / ODBC Linux still crashes here?

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