Skip to content

SQLColAttribute implementation - #68

Merged
alinaliBQ merged 5 commits into
apache-odbcfrom
sql-col-attribute
Jul 24, 2025
Merged

SQLColAttribute implementation#68
alinaliBQ merged 5 commits into
apache-odbcfrom
sql-col-attribute

Conversation

@alinaliBQ

@alinaliBQ alinaliBQ commented Jul 22, 2025

Copy link
Copy Markdown

SQLColAttribute implementation and tests.

  • Fixed bug with incorrect column count. It was returning column count + 1.
  • Fixed bug with incorrect numPrecRadix. It was returning SQL_NO_TOTAL (maps to -4) for non-numeric columns, but the correct value is 0.
  • Fixed bug with unsigned column to return true for unsigned columns (non-numeric columns or unsigned numeric columns) and false for signed numeric columns. It was returning the opposite.
  • Fixed bug with incorrect non-concise data type return value for date time type types. The correct return is SQL_DATETIME for all date time types

@alinaliBQ
alinaliBQ force-pushed the sql-col-attribute branch from 35887e5 to b9ed5f7 Compare July 22, 2025 23:37
@alinaliBQ
alinaliBQ force-pushed the sql-col-attribute branch from b9ed5f7 to 9d7661e Compare July 22, 2025 23:45
@alinaliBQ
alinaliBQ changed the base branch from sql-columns to apache-odbc July 22, 2025 23:45
@alinaliBQ
alinaliBQ force-pushed the sql-col-attribute branch 5 times, most recently from e14177e to 3e1002a Compare July 23, 2025 20:36
* Fixed bug with incorrect column count. It was returning column count + 1.
* Fixed bug with incorrect numPrecRadix. It was returning SQL_NO_TOTAL (maps to -4) for non-numeric columns, but the correct value is 0.
* Fixed bug with unsigned column to return true for unsigned columns (non-numeric columns or unsigned numeric columns) and false for signed columns (numeric columns)
* Fixed bug with incorrect non-concise data type return value for date time type types. The correct return is SQL_DATETIME for all date time types
@alinaliBQ
alinaliBQ force-pushed the sql-col-attribute branch from 3e1002a to b78bc31 Compare July 23, 2025 20:52
@alinaliBQ
alinaliBQ marked this pull request as ready for review July 23, 2025 21:07
ODBCStatement* statement = reinterpret_cast<ODBCStatement*>(stmt);
ODBCDescriptor* ird = statement->GetIRD();
SQLINTEGER outputLengthInt;
switch (fieldIdentifier) {

@jduo jduo Jul 23, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SQLColAttribute needs to handle the special SQL_COLUMN_* identifiers. I think these were for ODBC 2.x. Please test these as well.

@alinaliBQ alinaliBQ Jul 23, 2025

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.

Yup. I have added implementation for SQL_COLUMN_LENGTH, SQL_COLUMN_PRECISION, and SQL_COLUMN_SCALE, and made them return the same values as their corresponding SQL_DESC_* identifiers. For the rest of the SQL_COLUMN_* identifiers, they are the same values as the corresponding SQL_DESC_* identifiers, and the driver manager will map them accordingly. I have added ODBC Ver2 tests with SQLColAttributes that pass SQL_COLUMN_* identifiers as arguments.

Comment thread cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/odbc_api.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/odbc_api.cc Outdated
* use const for `ConvertToWString`
* add nullptr check in odbc_api.cc layer
* Add support for `SQL_COLUMN_LENGTH`, `SQL_COLUMN_PRECISION`, and `SQL_COLUMN_SCALE`. The driver will return the same values for ODBC2 and ODBC3.
* Add tests for ODBC v2 SQLColAttributes values
* I converted tests that can be run on remote servers where possible

@alinaliBQ alinaliBQ left a comment

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.

Addressed comments from James

Comment thread cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/odbc_api.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/odbc_api.cc Outdated
ODBCStatement* statement = reinterpret_cast<ODBCStatement*>(stmt);
ODBCDescriptor* ird = statement->GetIRD();
SQLINTEGER outputLengthInt;
switch (fieldIdentifier) {

@alinaliBQ alinaliBQ Jul 23, 2025

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.

Yup. I have added implementation for SQL_COLUMN_LENGTH, SQL_COLUMN_PRECISION, and SQL_COLUMN_SCALE, and made them return the same values as their corresponding SQL_DESC_* identifiers. For the rest of the SQL_COLUMN_* identifiers, they are the same values as the corresponding SQL_DESC_* identifiers, and the driver manager will map them accordingly. I have added ODBC Ver2 tests with SQLColAttributes that pass SQL_COLUMN_* identifiers as arguments.

Comment thread cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_metadata.cc Outdated
@alinaliBQ
alinaliBQ merged commit a7d97d5 into apache-odbc Jul 24, 2025
24 checks passed
@alinaliBQ
alinaliBQ deleted the sql-col-attribute branch July 24, 2025 17:56
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.

3 participants