Conversation
55c684f to
fbce21a
Compare
fbce21a to
8433b4f
Compare
| } | ||
| LOG(ERROR) << "WriteRowset::FetchNextResultSet:: " | ||
| << next_page_status.message; | ||
| if (row_counter == 0) { |
There was a problem hiding this comment.
Why do we return error only if row_count == 0? What if we fail to load second page?
| if (updated_rs.rows.empty()) { | ||
| break; | ||
| } | ||
| updated_rs.cursor++; |
There was a problem hiding this comment.
Why moving cursor without doing anything? Won't we miss 1 row of results due to this?
There was a problem hiding this comment.
We initialize the cursor by "-1" since in ODBC, broadly speaking, subsequent operations after SQLFetch on the current row (such as SQLGetData) expect the cursor to point to the active/last fetched row.
Ref: "When the result set is created, the cursor is positioned before the start of the result set."
| updated_rs.cursor++; | ||
| } | ||
|
|
||
| ResultSet& current_rs = stmt_handle.GetResultSet(); |
There was a problem hiding this comment.
Do we need 3 different vars? Why can't we just use original result_set?
Also we call GetResultSet() for each row which seems exceesive. We can initialize it prior to the 'while' loop & just update as needed.
| return status_record; | ||
| } | ||
|
|
||
| if (row_status_ptr) { |
There was a problem hiding this comment.
nit: do we need this check? Is it allowed to be null?
There was a problem hiding this comment.
Yes, it is null by default. SQL_ATTR_ROW_STATUS_PTR is optional.
| } | ||
|
|
||
| ResultSet& final_rs = stmt_handle.GetResultSet(); | ||
| if (row_counter > 0) { |
There was a problem hiding this comment.
Why do we need this? Can you add comment?
There was a problem hiding this comment.
Can we initialize array with SQL_ROW_NOROW & update it as we progress instead?
There was a problem hiding this comment.
Done in the latest commit.
8433b4f to
504f41a
Compare
2 participants
Summary
When an ODBC application configures block cursor / array fetching via
SQL_ATTR_ROW_ARRAY_SIZE(e.g., 5000 rows),SQLFetchandSQLFetchScrollpreviously only returned rows from the currentResultSetpage. If a query result set spanned multiple pages or streams (e.g., where each page returns 1000 rows),SQLFetchstopped at the end of the first page instead of continuing to fetch across page boundaries to satisfy the requested array size.This PR introduces a multi-page aware
WriteRowset(StatementHandle&, ...)that iterates across pages viaFetchNextResultSetuntil the requested rowset size is populated or no further data remains (SQL_NO_DATA).Key Changes
google/cloud/odbc/bq_driver/internal/odbc_sql_fetch.h&odbc_sql_fetch.cc:WriteRowset(StatementHandle& stmt_handle, int const rowset_size, DescriptorHandle& ard, DescriptorHandle& ird).FetchNextResultSet(stmt_handle)when reaching the end of the current page untilrow_counter == rowset_sizeor EOF.array_status_ptr(SQL_ROW_SUCCESS/SQL_ROW_NOROW),rows_processed_ptr, and cursor position.SQL_NO_DATAwhen reaching end of data during pagination.google/cloud/odbc/bq_driver/odbc_sql_results.cc:SQLFetchInternalandSQLFetchScrollInternalto delegate toWriteRowset(handle, ...).google/cloud/odbc/integration_tests/odbc_driver_tests/statement_test.cc:TEST_P(HTAPIParameterizedTest, SQLExecDirect_pagination_with_row_array_size)to verify that 3000 rows across page boundaries are fetched in 1 fetch call whenROW_ARRAY_SIZEis set to 5000.