Skip to content

Commit 5f45f56

Browse files
ai: apply changes for #1007 (1 review thread)
Addresses: - #4418605049 at src/databricks/sql/result_set.py:224 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent af2c1fc commit 5f45f56

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

‎src/databricks/sql/result_set.py‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,24 @@ def _fetchone_buffered(
220220
# still equals the queue position. Rolling the index back would make it
221221
# lag the queue and break Thrift's expected_row_start_offset check on
222222
# the next fetch, and retrying convert() on the same data would just
223-
# fail again.
224-
rows = convert(raw)
223+
# fail again. A warning records how many rows were discarded so a
224+
# caller that swallows the error and keeps fetching isn't silently
225+
# short of rows.
226+
try:
227+
rows = convert(raw)
228+
except Exception:
229+
if raw is None:
230+
dropped = 0
231+
elif isinstance(raw, list):
232+
dropped = len(raw)
233+
else:
234+
dropped = getattr(raw, "num_rows", 0)
235+
logger.warning(
236+
"fetchone(): failed to convert a batch of %d row(s); these rows "
237+
"are discarded and will not be returned by later fetches",
238+
dropped,
239+
)
240+
raise
225241
if not rows:
226242
return None
227243
self._row_buffer_raw = raw

‎tests/unit/test_fetches.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,23 @@ def test_mixed_fetches_keep_order_and_rownumber(self):
534534
self.assertEqual(rs.rownumber, 10)
535535
self.assertIsNone(rs.fetchone())
536536

537+
def test_fetchone_convert_failure_logs_dropped_rows_and_reraises(self):
538+
from unittest.mock import patch
539+
540+
rs = self.make_columnar_result_set(10, arraysize=4)
541+
with patch.object(
542+
ThriftResultSet,
543+
"_convert_columnar_table",
544+
side_effect=ValueError("bad value"),
545+
), self.assertLogs("databricks.sql.result_set", level="WARNING") as logs:
546+
with self.assertRaises(ValueError):
547+
rs.fetchone()
548+
self.assertTrue(any("batch of 4 row(s)" in m for m in logs.output))
549+
# The failed batch is dropped; the next fetch resumes after it.
550+
self.assertEqual(rs._row_buffer_count(), 0)
551+
self.assertEqual(rs.rownumber, 4)
552+
self.assertEqual(rs.fetchone()[0], 4)
553+
537554
@pytest.mark.skipif(pa is None, reason="PyArrow is not installed")
538555
def test_fetchone_then_fetchall_arrow_keeps_order(self):
539556
rs = self.make_columnar_result_set(10, arraysize=4)

0 commit comments

Comments
 (0)