Skip to content

Commit 4a8e377

Browse files
vuanhphungIsaac
andcommitted
test(thrift): check C-encoded requests match pure Python; qualify buffer bound
TBinaryProtocolAccelerated also moves request encoding to C. Add a test (skipped without fastbinary) that builds OpenSession, ExecuteStatement (with native parameters and query tags), GetTables and GetColumns requests through the backend and checks both encoders produce identical bytes. Clarify that the buffered reply is bounded by maxBytes only for fetches. Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 4ab0c54 commit 4a8e377

2 files changed

Lines changed: 92 additions & 2 deletions

File tree

‎src/databricks/sql/auth/thrift_http_client.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,8 @@ def flush(self):
221221

222222
# Thrift decodes the reply one field at a time; reading each field
223223
# straight from urllib3 costs far more than the decode itself. The
224-
# trade-off: the raw body (bounded by the request's maxBytes) is held
225-
# alongside the decoded objects until close() at the end of the RPC.
224+
# trade-off: the raw body is held alongside the decoded objects until
225+
# close() at the end of the RPC; for fetches it is bounded by maxBytes.
226226
self.__rbuf = BytesIO(self.__resp.read())
227227

228228
logger.info(

‎tests/unit/test_thrift_backend.py‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,96 @@ def test_reply_buffer_is_released_after_successful_make_request(self):
509509
self.assertEqual(result.results.arrowBatches[0].batch, batch)
510510
self.assertEqual(thrift_backend._transport.cstringio_buf.getvalue(), b"")
511511

512+
@unittest.skipUnless(
513+
_HAS_THRIFT_FASTBINARY, "thrift C extension (fastbinary) not built"
514+
)
515+
@patch("databricks.sql.auth.thrift_http_client.THttpClient")
516+
def test_accelerated_protocol_encodes_backend_requests_identically(
517+
self, t_http_client_class
518+
):
519+
"""TBinaryProtocolAccelerated also moves request encoding to C. Requests
520+
built by the backend must serialize byte-for-byte as with pure Python."""
521+
import datetime
522+
from thrift.protocol import TBinaryProtocol
523+
from thrift.transport import TTransport
524+
from databricks.sql.parameters import native
525+
526+
class Captured(Exception):
527+
pass
528+
529+
captured = []
530+
531+
def capture(method, request, *args, **kwargs):
532+
captured.append(request)
533+
raise Captured()
534+
535+
thrift_backend = ThriftDatabricksClient(
536+
"foobar",
537+
443,
538+
"path",
539+
[],
540+
auth_provider=AuthProvider(),
541+
ssl_options=SSLOptions(),
542+
http_client=MagicMock(),
543+
)
544+
thrift_backend.make_request = capture
545+
session_id = SessionId.from_thrift_handle(
546+
ttypes.TSessionHandle(
547+
sessionId=ttypes.THandleIdentifier(guid=b"g" * 16, secret=b"s" * 16)
548+
)
549+
)
550+
params = [
551+
native.IntegerParameter(value=42, name="i"),
552+
native.BigIntegerParameter(value=2**40, name="b"),
553+
native.StringParameter(value="héllo", name="s"),
554+
native.DecimalParameter(value=Decimal("12.345"), name="d"),
555+
native.TimestampParameter(
556+
value=datetime.datetime(2026, 11, 11, 1, 2, 3), name="t"
557+
),
558+
native.DateParameter(value=datetime.date(2026, 11, 11), name="dt"),
559+
native.BooleanParameter(value=True, name="bo"),
560+
native.VoidParameter(value=None, name="v"),
561+
native.ArrayParameter(value=[1, 2, 3], name="a"),
562+
native.MapParameter(value={"k": "v"}, name="m"),
563+
]
564+
calls = [
565+
lambda: thrift_backend.open_session(
566+
{"use_cached_result": False, "ansi_mode": "true"}, "main", "default"
567+
),
568+
lambda: thrift_backend.execute_command(
569+
operation="SELECT :i, :s",
570+
session_id=session_id,
571+
max_rows=100000,
572+
max_bytes=104857600,
573+
lz4_compression=True,
574+
cursor=Mock(),
575+
use_cloud_fetch=True,
576+
parameters=[p.as_tspark_param(named=True) for p in params],
577+
async_op=False,
578+
enforce_embedded_schema_correctness=False,
579+
row_limit=10,
580+
query_tags={"team": "peco", "flag": None},
581+
),
582+
lambda: thrift_backend.get_tables(
583+
session_id, 100000, 104857600, Mock(), "main", "%", "t%", ["TABLE"]
584+
),
585+
lambda: thrift_backend.get_columns(
586+
session_id, 100000, 104857600, Mock(), "main", "%", "%", "%"
587+
),
588+
]
589+
for call in calls:
590+
with self.assertRaises(Captured):
591+
call()
592+
593+
self.assertEqual(len(captured), len(calls))
594+
for request in captured:
595+
pure, accelerated = TTransport.TMemoryBuffer(), TTransport.TMemoryBuffer()
596+
request.write(TBinaryProtocol.TBinaryProtocol(pure))
597+
request.write(TBinaryProtocol.TBinaryProtocolAccelerated(accelerated))
598+
self.assertEqual(
599+
accelerated.getvalue(), pure.getvalue(), type(request).__name__
600+
)
601+
512602
@patch("databricks.sql.backend.thrift_backend.unsafe_logger")
513603
@patch("databricks.sql.auth.thrift_http_client.THttpClient")
514604
def test_unsafe_logger_formats_payloads_lazily(

0 commit comments

Comments
 (0)