Skip to content

SQLColumns Implementation - #66

Merged
alinaliBQ merged 18 commits into
apache-odbcfrom
sql-columns
Jul 22, 2025
Merged

SQLColumns Implementation#66
alinaliBQ merged 18 commits into
apache-odbcfrom
sql-columns

Conversation

@alinaliBQ

Copy link
Copy Markdown

No description provided.

@alinaliBQ
alinaliBQ marked this pull request as ready for review July 18, 2025 20:13

add_arrow_test(flight_sql_odbc_test
SOURCES
columns_test.cc

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SQLColumns is one of the functions that behaves differently in ODBC 2.x vs. ODBC 3.x and up. The SQL type it returns for datetime types changes depending on whether you're using ODBC 2 or 3. For example SQL_TIME=11 vs. SQL_TYPE_TIME=91 (or something similar to this).

Also the column names differ in 2 vs. 3:
TABLE_QUALIFIER in 2 and TABLE_CAT in 3.

This is also the case for SQLTables.

Please add a test for ODBC 2.x

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.

For the column names difference, we will add a test as part of SQLDescribeCol implementation in milestone 3. Yes the ODBC2 vs. 3 is pretty important, I remember the macOS Excel (which uses ODBC2) breaking and not getting database names because a driver I worked on before was returning TABLE_CAT as column name.

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.

Will look for a way to test time types with remote server

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.

Found the diff on odbc 2 and odbc 3:

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.

I have added the test for different data type return value for SQLColumns, and added todo comment for SQLDescribeCol tests

#include "gtest/gtest.h"

namespace arrow::flight::sql::odbc {
void checkSQLColumns(

@jduo jduo Jul 19, 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.

We need to validate the metadata of the SQLColumns result itself too (ie run SQLDescribeCol and check the column names, the number of columns, types for each column, etc). This needs to be done at least once for each of ODBC 2 and 3.

This applies to other catalog functions.

@alinaliBQ alinaliBQ Jul 21, 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.

Sure, we can add the tests for this as part of SQLDescribeCol implementation, SQLDescribeCol is scheduled in milestone 3 so we can add the metadata tests for SQLColumns and SQLTables in milestone 3. (Btw SQLPrimaryKeys and SQLForeignKeys were not implemented in the flightsql-odbc in the apache arrow repo https://github.com/apache/arrow, so would be out of scope for us to implement)

- Test different data type return value for SQLColumns
- Add todo comment for SQLDescribeCol tests
@alinaliBQ
alinaliBQ merged commit c03ed46 into apache-odbc Jul 22, 2025
24 checks passed
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