Develop utility getters for ODBC APIs#136
Merged
alinaliBQ merged 7 commits intoapache-odbcfrom Nov 7, 2025
Merged
Conversation
Need to verify the development with community
Types added: - SQLUSMALLINT - SQLUINTEGER - SQLULEN - SQLWCHAR Prototype `TestSQLGetInfoNullCollation` test
alinaliBQ
force-pushed
the
getter-tests
branch
2 times, most recently
from
a5b5cf1 to
d071319
Compare
- string getter - number getter Plus renaming tests SQLColAttribute prototype `TestSQLColAttributeBasetable_name`
alinaliBQ
force-pushed
the
getter-tests
branch
from
d071319 to
77381b5
Compare
justing-bq
reviewed
Nov 7, 2025
| void CheckSQLColAttributeString(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, | ||
| SQLUSMALLINT field_identifier, | ||
| const std::wstring& expected_attr_string) { | ||
| void getSQLColAttributeString(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, |
| void CheckSQLColAttributesString(SQLHSTMT stmt, const std::wstring& wsql, | ||
| SQLUSMALLINT idx, SQLUSMALLINT field_identifier, | ||
| const std::wstring& expected_attr_string) { | ||
| void getSQLColAttributesString(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, |
| CheckSQLColAttributeNumeric(this->stmt, wsql, 1, SQL_DESC_CASE_SENSITIVE, SQL_FALSE); | ||
| SQLLEN value; | ||
| getSQLColAttributeNumeric(this->stmt, wsql, 1, SQL_DESC_CASE_SENSITIVE, &value); | ||
| ASSERT_EQ(SQL_FALSE, value); |
There was a problem hiding this comment.
Before we had the assertion with the expected value inside CheckSQLColAttributeNumeric() and now it's pulled out. Any particular reason for that? Perhaps it's still worth having a utility function that handles the assertion?
Likewise for all the other spots where this is happening.
Author
There was a problem hiding this comment.
I'm pulling out the assertions due to community comment from David: apache#47762 (comment)
alinaliBQ
commented
Nov 7, 2025
Author
alinaliBQ
left a comment
alinaliBQ
left a comment
There was a problem hiding this comment.
Addressed comments from Justin
| void CheckSQLColAttributeString(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, | ||
| SQLUSMALLINT field_identifier, | ||
| const std::wstring& expected_attr_string) { | ||
| void getSQLColAttributeString(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, |
| void CheckSQLColAttributesString(SQLHSTMT stmt, const std::wstring& wsql, | ||
| SQLUSMALLINT idx, SQLUSMALLINT field_identifier, | ||
| const std::wstring& expected_attr_string) { | ||
| void getSQLColAttributesString(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, |
justing-bq
reviewed
Nov 7, 2025
| SQLUSMALLINT idx, SQLUSMALLINT field_identifier, | ||
| SQLLEN expected_attr_numeric) { | ||
| // Execute query and check ODBC 2.0 API SQLColAttributes numeric attribute | ||
| void getSQLColAttributeNumeric(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, |
There was a problem hiding this comment.
Still needs to be updated to PascalCase.
Author
There was a problem hiding this comment.
yes, I missed it, my bad. It is fixed
| ASSERT_EQ(SQL_SUCCESS, SQLColAttribute(stmt, idx, field_identifier, 0, 0, 0, value)); | ||
| } | ||
|
|
||
| void getSQLColAttributesNumeric(SQLHSTMT stmt, const std::wstring& wsql, SQLUSMALLINT idx, |
justing-bq
approved these changes
Nov 7, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
create utility getters and then using standard GTest assertions, instead of making a function for every combination of data type and comparator.
Added getters for:
SQLGetStmtAttr
SQLGetInfo
SQLColAttribute / SQLColAttributes (with s at the end)
Removed disabled tests for
SQLGetInfo