[7/7] Enable insert batching by default - #1634
Conversation
Keep existing batching behavior behind a dedicated executor so native routing can be added without changing the compatibility fallback. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Introduce a dormant opt-in and deterministic parameter snapshots so both native transports can share the same ordered payload without changing execution behavior. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Snapshot prepared parameter sets at addBatch time and route eligible batches through an injectable native executor while preserving the legacy production path. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Accept the immutable parameter map produced by the shared batch model instead of requiring a concrete HashMap. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Route eligible prepared batches through compute-aware Thrift batchParameters, preserve legacy compatibility fallback, and map repeat-compressed backend results to ordered JDBC update counts. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Surface post-success count-read errors as RESULT_SET_ERROR and clear committed batches before propagating so retries cannot duplicate inserted rows. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Submit ordered parameter_sets through the existing SEA execution lifecycle so opt-in prepared batches share the same routing, fallback, and JDBC result handling as Thrift. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Record real SEA and Thrift responses to guard native parameter batching end to end. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Validate exact parameter values and array order so replay coverage detects reordered native batches. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Activate native server-side batching while enabling the legacy rewrite fallback for older compute, with explicit kill switches for both paths. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Document the DBR version requirement, backend limits, and legacy configuration for binary batches. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Clean activation PR — verified the native/legacy routing so enabling both defaults is safe (native prioritized, legacy fallback, no double execution) and the test updates match the new defaults. One low note on the legacy fallback's hardcoded per-row update counts becoming the default behavior. Reminder (already documented by the author): this is intentionally a draft and must not merge until native BINARY parameter support lands on SEA and Thrift.
Scope: reviewed as a stacked increment (07968b8..head); the parent's changes are reviewed on the parent PR.
| "Timeout in seconds for metadata polling operations (e.g. GetTables, GetColumns). 0 means no timeout", | ||
| "300"), | ||
| ENABLE_BATCHED_INSERTS("EnableBatchedInserts", "Enable batched INSERT optimization", "0"), | ||
| ENABLE_BATCHED_INSERTS("EnableBatchedInserts", "Enable batched INSERT optimization", "1"), |
There was a problem hiding this comment.
🔵 Low — Flipping EnableBatchedInserts to 1 by default turns on the legacy client-side multi-row rewrite for the DBR < 18.2 fallback path. Note that this path does not read real per-row update counts from the server — executeBatchedInsert hardcodes allUpdateCounts[i] = 1 for every row (see the loop that sets each chunk's counts). So on the new default, callers that inspect executeBatch() return values will always see 1 per row rather than the actual affected-row count, whereas the previous default (batched inserts off) returned the server's real getUpdateCount() per statement. This is the intended design of the batching feature, but it is a user-visible behavior change worth calling out explicitly in the changelog entry (which currently only describes native batching's parameter/size limits, not the update-count semantics of the legacy fallback).
🥞 Stacked PR
Use this link to review incremental changes.
Description
PreparedStatementINSERT batches on DBR 18.2 and later.EnableNativeBatching=0andEnableBatchedInserts=0as independent kill switches.BINARYlimitation.Testing
Additional Notes to the Reviewer
BINARYvalues. Binary batches must use legacy client-side batching withEnableNativeBatching=0andsupportManyParameters=1(EnableBatchedInsertsdefaults to1).1for each submitted row because the backend returns a count for the combined multi-row statement rather than separate per-row counts.