Skip to content

[7/7] Enable insert batching by default - #1634

Open
sreekanth-db wants to merge 11 commits into
mainfrom
sreekanth-db/stack/native-batch-activation
Open

[7/7] Enable insert batching by default#1634
sreekanth-db wants to merge 11 commits into
mainfrom
sreekanth-db/stack/native-batch-activation

Conversation

@sreekanth-db

@sreekanth-db sreekanth-db commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

🥞 Stacked PR

Use this link to review incremental changes.


Description

  • Enable native server-side batching by default for supported PreparedStatement INSERT batches on DBR 18.2 and later.
  • Enable the legacy client-side multi-row rewrite by default for older DBR versions and compatibility fallback.
  • Keep EnableNativeBatching=0 and EnableBatchedInserts=0 as independent kill switches.
  • Report JDBC batch support when either strategy is enabled.
  • Document the 10,000-parameter / 1 MB limits and native BINARY limitation.

Testing

  • Focused connection-context, metadata, routing, and prepared-statement unit tests.
  • SEA and Thrift fake-service replay tests using real recorded responses.
  • Live SEA and Thrift warehouse validation for native and legacy batching branches, limits, transactions, data types, and statement lifecycle.
  • Full PR CI across supported JDK and operating-system combinations.

Additional Notes to the Reviewer

  • Native batching does not support BINARY values. Binary batches must use legacy client-side batching with EnableNativeBatching=0 and supportManyParameters=1 (EnableBatchedInserts defaults to 1).
  • After a successful legacy rewrite chunk, JDBC reports update count 1 for each submitted row because the backend returns a count for the combined multi-row statement rather than separate per-row counts.

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>
@sreekanth-db sreekanth-db changed the title Enable insert batching by default [7/7] Enable insert batching by default Aug 11, 2026
Document the DBR version requirement, backend limits, and legacy configuration for binary batches.

Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
@sreekanth-db
sreekanth-db marked this pull request as ready for review August 12, 2026 11:05

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant