Skip to content

Speed up Cursor.fetchall by draining batches directly #196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TheDistributor wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Speed up Cursor.fetchall by draining batches directly #196

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e934fa6
Optimize result fetching via block-draining and check inlining
Jun 29, 2026
8fe09f5
merge fetchmany and fetchall
Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions pynuodb/cursor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,11 @@ def executemany(self, operation, seq_of_parameters):
def fetchone(self):
# type: () -> Optional[result_set.Row]
"""Return the next row of results from the previous SQL operation."""
self._check_closed()
# Inline _check_closed to avoid per-row function-call overhead.
if self.closed:
raise Error("cursor is closed")
if self.session.closed:
raise Error("connection is closed")
Comment on lines -187 to +191

@madscientist madscientist Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this buying us enough to justify duplicating the code? I know method calls in python are slower but IMO we need to weigh the overhead of supporting multiple copies of the same code and the possibilities for behavior divergence.

My opinion is that no one who chooses to use the python driver is going to be fussed about an extra 50 nanoseconds of speed or whatever, even in a per-row basis. And if they do care about it then we should be working on using a compiled solution rather than a native python solution.

The change below to fetchall() is great. But to me this one feels like chasing cycles at the expense of code maintainability and IMO that's not the way we should be leaning, for a Python implementation.

if self._result_set is None:
raise Error("Previous execute did not produce any results or no call was issued yet")
self.rownumber += 1
Expand Down Expand Up @@ -218,14 +222,22 @@ def fetchall(self):
# type: () -> List[result_set.Row]
"""Return all rows generated by the previous SQL operation."""
self._check_closed()
if self._result_set is None:
raise Error("Previous execute did not produce any results or no call was issued yet")

fetched_rows = []
fetched_rows = [] # type: List[result_set.Row]
while True:
row = self.fetchone()
if row is None:
# Drain the current in-memory batch in one shot instead of calling
# fetchone() per row.
idx = self._result_set.results_idx
batch = self._result_set.results
if idx < len(batch):
fetched_rows.extend(batch[idx:])
self._result_set.results_idx = len(batch)
if self._result_set.complete:
break
else:
fetched_rows.append(row)
self.session.fetch_result_set_next(self._result_set)
self.rownumber += len(fetched_rows)
return fetched_rows

def nextset(self): # pylint: disable=no-self-use
Expand Down