Skip to content

Resolve ODBC CI Build Errors - #129

Merged
alinaliBQ merged 2 commits into
apache-odbcfrom
fix-dsnconfigw
Oct 27, 2025
Merged

Resolve ODBC CI Build Errors#129
alinaliBQ merged 2 commits into
apache-odbcfrom
fix-dsnconfigw

Conversation

@alinaliBQ

Copy link
Copy Markdown
  • Fix ConfigDSNW not found build issue
  • Fix TestFreeNullHandles test
    ConnectionTest sets up the handles, so the this->... handles will not be null at beginning of test.

`ConnectionTest` sets up the handles, so the `this->...` handles will not be null at beginning of test.
}
} // namespace arrow::flight::sql::odbc

BOOL INSTAPI ConfigDSNW(HWND hwnd_parent, WORD req, LPCWSTR wdriver,

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.

If ConfigDSNW is under namespace arrow::flight::sql::odbc, then ConfigDSNW is defined as arrow::flight::sql::odbc::ConfigDSNW which is not recognized by stricted compilers. So ConfigDSNW needs to be defined on its own without any namespace

@alinaliBQ
alinaliBQ marked this pull request as ready for review October 27, 2025 20:10
TEST(SQLFreeHandle, TestFreeNullHandles) {
SQLHENV env = NULL;
SQLHDBC conn = NULL;
SQLHSTMT stmt = NULL;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this change necessary?

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.

The TestFreeNullHandles test was failing for me, ConnectionTest fixture sets up the handles, so the this->... handles will not be null at the beginning of test. I think originally TestFreeNullHandles wasn't a part of the ConnectionTest fixture.

@alinaliBQ
alinaliBQ merged commit c04a78f into apache-odbc Oct 27, 2025
11 of 12 checks passed
@alinaliBQ
alinaliBQ deleted the fix-dsnconfigw branch October 27, 2025 21:01
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.

2 participants