GH-49482: [C++][FlightRPC][ODBC] Fix inconsistent SQLGetInfo values in global connection by vanshaj2023 · Pull Request #50021 · apache/arrow

vanshaj2023 · 2026-05-22T19:04:01Z

What changed

SQL_DDL_SCHEMA case now reads the boolean scalar to conditionally set SQL_DROP_SCHEMA and SQL_CREATE_SCHEMA to 0 when unsupported, instead of always writing non-zero values
SQL_DDL_TABLE case applies the same pattern for SQL_DROP_TABLE and SQL_CREATE_TABLE
SQL_CATALOG_AT_START changed to use SetDefaultIfMissing so it does not overwrite SQL_CATALOG_LOCATION already set by ARROW_SQL_CATALOG_TERM, eliminating an ordering-dependent conflict
Removed per-test connect/disconnect workarounds and converted three tests from ConnectionInfoHandleTest back to ConnectionInfoTest

How to test

Build ODBC tests and run TestSQLGetInfoCatalogLocation, TestSQLGetInfoDropSchema, TestSQLGetInfoDropTable with both mock server and global connection fixture
All three tests should pass consistently regardless of connection reuse

Closes #49482

GitHub Issue: [C++][FlightRPC][ODBC] Fix inconsistent values in SQLGetInfo in global connection #49482

lidavidm

Thanks for the PR!

Also CC @alinaliBQ

lidavidm · 2026-05-25T06:34:58Z

+              bool supported =
+                  reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;


Maybe we can use checked_cast from arrow/util/checked_cast.h?

lidavidm · 2026-05-25T06:35:34Z

It seems there's some failing tests - some more test cases need updating?

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Resolves inconsistent return values for several ODBC connection info properties (SQL_CATALOG_LOCATION, SQL_DROP_SCHEMA, SQL_DROP_TABLE) by properly deriving them from Flight SQL boolean info responses, and replaces unchecked reinterpret_cast with checked_cast for scalar conversions.

Changes:

Use scalar boolean values from SQL_DDL_SCHEMA and SQL_DDL_TABLE to determine SQL_DROP_SCHEMA, SQL_CREATE_SCHEMA, SQL_DROP_TABLE, and SQL_CREATE_TABLE.
Avoid overwriting SQL_CATALOG_LOCATION when previously set, by using SetDefaultIfMissing.
Replace reinterpret_cast<BooleanScalar*> with checked_cast<BooleanScalar*> and update affected tests.

Reviewed changes

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

File	Description
cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc	Derives DDL-related info values from server-provided booleans, guards `SQL_CATALOG_LOCATION` against overwrite, and switches to `checked_cast`.
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc	Updates tests to reflect corrected behavior, moving some tests from `ConnectionInfoHandleTest` to `ConnectionInfoTest`/`ConnectionInfoMockTest`.

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

vanshaj2023 · 2026-05-25T14:54:48Z

Hi @lidavidm,
Thanks for the review! I've addressed both comments switched to checked_cast and updated the DropSchema/DropTable test expectations to SQL_DS_DROP_SCHEMA/SQL_DT_DROP_TABLE since the mock sends true for both DDL flags. CI failures are a pre-existing flake unrelated to this change.

Copilot

Pull request overview

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

alinaliBQ

Hi @vanshaj2023 thanks for raising the PR. Please build and test ODBC locally first.

FYI - Once these 2 PRs are merged, ODBC will be tested against a Dremio instance in the Linux CI. This might be helpful.

alinaliBQ · 2026-05-25T17:23:07Z


-TYPED_TEST(ConnectionInfoHandleTest, TestSQLGetInfoDropSchema) {
-  // GH-49482 TODO: resolve inconsitent return value for SQL_DROP_SCHEMA and change test
-  // type to `ConnectionInfoTest`
-  this->ConnectWithString(this->GetConnectionString(), this->conn);
-
+TEST_F(ConnectionInfoMockTest, TestSQLGetInfoDropSchema) {


Could you use TYPED_TEST(ConnectionInfoTest, here to be consistent with rest of code base?

alinaliBQ · 2026-05-25T17:23:14Z

-  // type to `ConnectionInfoTest`
-  this->ConnectWithString(this->GetConnectionString(), this->conn);
-
+TEST_F(ConnectionInfoMockTest, TestSQLGetInfoDropTable) {


Could you use TYPED_TEST(ConnectionInfoTest, here to be consistent with rest of code base?

alinaliBQ · 2026-05-25T17:29:51Z

@vanshaj2023 To ensure maximum coverage, please build and run the ODBC tests locally against a Dremio docker instance with documentation here:

arrow/cpp/src/arrow/flight/sql/odbc/README.md

Lines 88 to 89 in cd1811b

## Steps to Run the ODBC tests

After ODBC has been registered, you can run the ODBC tests. It is recommended to run the ODBC tests locally first.

alinaliBQ · 2026-05-25T17:31:42Z

              transactions_supported =
-                  reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;


Please fix the CI build failures from checked_cast in this file:
https://github.com/apache/arrow/actions/runs/26406439067/job/77730895486?pr=50021#step:7:4419

/Users/runner/work/arrow/arrow/cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc:499:19: error: use of undeclared identifier 'checked_cast'; did you mean 'internal::checked_cast'? 499 | checked_cast<BooleanScalar*>(scalar->child_value().get())->value; | ^~~~~~~~~~~~ | internal::checked_cast /Users/runner/work/arrow/arrow/cpp/src/arrow/util/checked_cast.h:28:19: note: 'internal::checked_cast' declared here

apacheGH-49482: fix inconsistent SQLGetInfo values in global connection

a355afd

vanshaj2023 requested a review from lidavidm as a code owner May 22, 2026 19:04

github-actions Bot added Component: C++ Component: FlightRPC CI: Extra: C++ Run extra C++ CI awaiting review Awaiting review labels May 22, 2026

lidavidm reviewed May 25, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels May 25, 2026

address review: use checked_cast and fix test expectations for DDL info

63e9ee9

Copilot AI review requested due to automatic review settings May 25, 2026 14:50

Copilot AI reviewed May 25, 2026

View reviewed changes

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels May 25, 2026

vanshaj2023 requested review from Copilot and lidavidm May 25, 2026 14:56

Copilot AI reviewed May 25, 2026

View reviewed changes

alinaliBQ reviewed May 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-49482: [C++][FlightRPC][ODBC] Fix inconsistent SQLGetInfo values in global connection#50021

GH-49482: [C++][FlightRPC][ODBC] Fix inconsistent SQLGetInfo values in global connection#50021
vanshaj2023 wants to merge 2 commits into
apache:mainfrom
vanshaj2023:fix/GH-49482-fix-sqlgetinfo-inconsistent-values

vanshaj2023 commented May 22, 2026 •

edited by github-actions Bot

Loading

Uh oh!

lidavidm left a comment

Uh oh!

lidavidm May 25, 2026

Uh oh!

lidavidm May 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

vanshaj2023 commented May 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

alinaliBQ left a comment •

edited

Loading

Uh oh!

alinaliBQ May 25, 2026

Uh oh!

alinaliBQ May 25, 2026

Uh oh!

alinaliBQ May 25, 2026

Uh oh!

alinaliBQ May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		bool supported =
		reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;

	## Steps to Run the ODBC tests
	After ODBC has been registered, you can run the ODBC tests. It is recommended to run the ODBC tests locally first.

		transactions_supported =
		reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;

Conversation

vanshaj2023 commented May 22, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

How to test

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

lidavidm May 25, 2026

Choose a reason for hiding this comment

Uh oh!

lidavidm May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

vanshaj2023 commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

alinaliBQ left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ May 25, 2026

Choose a reason for hiding this comment

Uh oh!

alinaliBQ May 25, 2026

Choose a reason for hiding this comment

Uh oh!

alinaliBQ May 25, 2026

Choose a reason for hiding this comment

Uh oh!

alinaliBQ May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vanshaj2023 commented May 22, 2026 •

edited by github-actions Bot

Loading

vanshaj2023 commented May 25, 2026 •

edited

Loading

alinaliBQ left a comment •

edited

Loading