SQLFetch & SQLGetData Implementation - #55
Conversation
Remove JSON and struct code Initial draft code SQLExecDirect + Example initial test SQLGetStmtAttr stub implementation Stub call for SQLGetStmtAttr Enable statement handle allocation Implement SQLExecDirect - test hang issue Update odbc_api.cc Run `ShutdownProtobufLibrary` after all ODBC tests Resolves test hanging problem Fix missing break statement in SQLGetDiagRec Add tests Lint fixes Run ShutdownProtobufLibrary as part of test environment Add debug messages Draft code Remove drafts Add comment Add comment for apacheGH-46889 Update statement test headers Pass call options to executed prepared statement `call_options_` contains the authentication token which is needed SQLFetch & SQLGetData initial impl EVERYTHING before this commit is for SQLExecDirect SQLFetch and SQLGetData TestSQLExecDirectSimpleQuery test Make GetData() return SQLRETURN `TestSQLExecDirectDataQuery` for remote and mock servers Remove unneeded test case Add more data types
Add varbinary test
Will leave tests for later
The `TestSQLExecDirectDataQuery` checks for date is not working.
* imported fix from dremio/flightsql-odbc@d44d862, an Apache-Licensed repository
| } | ||
|
|
||
| void ODBCStatement::getRowCount(SQLLEN* rowCountPtr) { | ||
| if (!rowCountPtr) { |
There was a problem hiding this comment.
I don't see in the spec this error mentioned, which means we can actually just return success and not do anything. Same with num result cols.
There was a problem hiding this comment.
got it. I have removed the thrown errors for SQLNumResultCols and SQLRowCount
| throw DriverException("Invalid null pointer for number of column results.", "HY000"); | ||
| } | ||
| size_t columnCount = m_currentArd->GetRecords().size(); | ||
| *columnCountPtr = columnCount; |
| /// \brief Get invalid connection string for mock server | ||
| std::string getInvalidConnectionString() override; | ||
| /// \brief Return a SQL query that selects all data types | ||
| std::wstring getQueryAllDataTypes() override; |
There was a problem hiding this comment.
We need to repeat these tests but use SQL_C_DEFAULT as the target type instead of the real SQL_C_* type. (Real applications like Excel use SQL_C_DEFAULT).
We should also test SQL_ARD_TYPE when descriptor functions are added such as SQLSetDescField.
There was a problem hiding this comment.
We need to validate that the indicator pointer gets set to the correct length (in bytes) for string and binary data. The driver is also allowed to return SQL_NO_TOTAL.
We need to check if a NULL cell correctly sets the indicator pointer to SQL_NULL_DATA.
We should handle test the case where the indicator pointer itself is null.
- If the cell is non-NULL this should succeed gracefully.
- If the cell is NULL there's an error that should get reported.
Test that when retrieving fixed-length data, if buffer length is set to a garbage value (like a negative number) it gets ignored.
We also need to test retrieving string and binary data in parts. When the buffer length is too small:
- Return SQL_SUCCESS_WITH_INFO with error state 01004
- I think the indicator pointer is supposed to hold the length of the remaining data after the current chunk. If the indicator pointer is NULL this should behave normally.
There was a problem hiding this comment.
Sounds good, today I have added tests for everything you mentioned except case:
Test that when retrieving fixed-length data, if buffer length is set to a garbage value (like a negative number) it gets ignored.
Will continue working on this on Friday
There was a problem hiding this comment.
I have added test for passing garbage value as buffer length
- do not return errors for invalid rowCountPtr and columnCountPtr - add static cast for columnCount - Add tests with SQL_C_DEFAULT as target type - Add test checks for indicator - Fix SQL_NUMERIC, SQL_BIT, SQL_FLOAT target type
alinaliBQ
left a comment
There was a problem hiding this comment.
Will continue addressing comments tmr
| } | ||
|
|
||
| void ODBCStatement::getRowCount(SQLLEN* rowCountPtr) { | ||
| if (!rowCountPtr) { |
There was a problem hiding this comment.
got it. I have removed the thrown errors for SQLNumResultCols and SQLRowCount
| throw DriverException("Invalid null pointer for number of column results.", "HY000"); | ||
| } | ||
| size_t columnCount = m_currentArd->GetRecords().size(); | ||
| *columnCountPtr = columnCount; |
| /// \brief Get invalid connection string for mock server | ||
| std::string getInvalidConnectionString() override; | ||
| /// \brief Return a SQL query that selects all data types | ||
| std::wstring getQueryAllDataTypes() override; |
There was a problem hiding this comment.
Sounds good, today I have added tests for everything you mentioned except case:
Test that when retrieving fixed-length data, if buffer length is set to a garbage value (like a negative number) it gets ignored.
Will continue working on this on Friday
|
The C++ / AMD64 Conda C++ AVX2 (pull_request) failures should not due to our code but rather something at Arrow's source |
SQLFetch & SQLGetData Implementation + Tests