Repository navigation
fix: convert rows in batches for fetchone() and cursor iteration - #982
Open
maharanay22 wants to merge 1 commit into
Open
maharanay22 wants to merge 1 commit into
maharanay22 wants to merge 1 commit into
Conversation
fetchone() fetched a single row and ran it through _convert_arrow_table, which builds a pandas DataFrame per call. That fixed cost made iterating a cursor, or any fetchone() loop such as SQLAlchemy's default result iteration, about 1 ms per row, more than 100x slower than fetchall(). ResultSet now implements fetchone(), fetchmany() and fetchall() once for all backends. fetchone() fetches and converts arraysize rows at a time and serves them from a small buffer that keeps both the converted rows and the raw batch. Every other fetch method drains that buffer first, so ordering and counts are unchanged when row and Arrow/columnar/JSON fetches are mixed, and rownumber still reports the number of rows handed to the caller. Signed-off-by: Maha Rana Yadavalli <271375718+maharanay22@users.noreply.github.com>
This branch has not been deployed
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
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.
What type of PR is this?
Description
Fixes #981.
Iterating a cursor (
for row in cursor) or callingfetchone()in a loop costs about 0.6 to 0.8 ms per row, more than 100x slower thanfetchall().fetchone()fetched a single row and passed it through_convert_arrow_table, which builds a pandas DataFrame on every call. SQLAlchemy's defaultResultiteration callscursor.fetchone()per row, so a 1M-row read through SQLAlchemy takes many minutes instead of secondsThis PR moves
fetchone(),fetchmany()andfetchall()into theResultSetbase class so Thrift, SEA and kernel share one implementation. Each backend now provides_fetchmany_table,_fetchall_tableand_convert_table.fetchone()fetches and convertsarraysizerows at a time and serves them from a small buffer that keeps both the converted rows and the raw batch. Every other fetch method (fetchmany,fetchall,fetchmany_arrow,fetchall_arrow, and the columnar/JSON variants) drains that buffer first, so ordering and counts stay exact when row and Arrow fetches are mixed, for examplefetchone()followed byfetchall_arrow().rownumberstill reports the rows handed to the caller. The buffer holds at most one batch ofarraysizerows and is released as soon as it is drainedTimings on 20,000 rows (bigint + string) from an in-memory Arrow queue:
One trade-off: the first
fetchone()now fills up toarraysizerows, so on Thrift it can make the nextFetchResultscall earlier than before when the direct results hold fewer rows than that. Loweringcursor.arraysizerestores the old behaviorHow is this tested?
New tests in
test_fetches.py,test_sea_result_set.pyandtest_kernel_result_set.pycount calls to the conversion function and check that it runs once per batch instead of once per row. These fail onmain(21, 6 and 10 calls instead of 3, 2 and 2) and pass with this change. Other new tests cover the interleavings:fetchone()thenfetchall(), iteration across several queue batches,fetchone()thenfetchmany()thenfetchall_arrow(),fetchmany_arrow()spanning buffered and new rows, ColumnQueue and JSON paths, empty results, andrownumberat each step.test_sea_result_set.py::test_fetchonenow asserts onrownumberinstead of the private_next_row_index. Full unit suite: 1045 passed, 5 skipped with pandas 3, and the touched suites pass with pandas 2.3.black --check srcandmypy srcpass. The timings above were measured manually on both pandas versionsRelated Tickets & Documents
Fixes #981. Affects every backend's
fetchone()and therefore SQLAlchemy result iteration