Describe the bug
QueryPlanSerde attaches a QueryContext to every expression it serializes, and QueryContext.sql_text is the full SQL text of the query. The same string is therefore embedded once per expression in the serialized native plan.
On a modest 2-table join + aggregate query (778-char SQL), 71 of the 90 Expr nodes in the plan carried a copy of the query text:
total serialized plan bytes = 58445
Expr nodes in plan = 90
Expr nodes with query_context = 71
query_context serialized bytes = 56227 (96.2% of plan)
distinct sql_text strings = 1
plan bytes with ctx stripped = 1803 <- 32x smaller
per-task proto round trip full = 0.153 ms
per-task proto round trip slim = 0.063 ms <- 2.4x faster
TPC-DS queries are 1-3 KB of SQL, so the ratio gets worse, not better.
Where it comes from
QueryPlanSerde.exprToProtoInternal ends with a .map that calls extractQueryContext(expr) for every converted expression; aggExprToProto does the same for aggregates.
QueryContext.sql_text in native/proto/src/proto/expr.proto is declared as the full query text, and SQLQueryContext.sqlText is what gets copied into it.
Why it costs more than just bytes
- Plan size. The
Array[Byte] is captured by CometExecRDD and shipped in the task binary for the stage.
- Per-task JVM protobuf round trip. Whenever a plan contains a native scan (i.e. essentially every scan-rooted Comet stage),
CometExecRDD.compute does Operator.parseFrom(serializedPlan) then PlanDataInjector.injectPlanData then serializeOperator — per task. Measured at 0.153 ms vs 0.063 ms with the duplication removed. At 10k tasks that is ~1.5 s of JVM protobuf churn per stage, ~60% of it avoidable.
- Native side.
PhysicalPlanner::create_expr and create_agg_expr do ctx_proto.sql_text.clone() into a fresh String and register it in QueryContextMap, per expression, per task. That is 71 identical heap strings per plan per task, held for the lifetime of the query. Note QueryContext.sql_text is already Arc<String> internally, so the sharing was anticipated — it is just never realised because each proto entry decodes to its own String.
Proposed fix: intern the SQL text into a pool
Truncating to just the relevant fragment is not an option: QueryContext::format_summary prints the entire sql_text (that is what Spark's == SQL (line N, position M) == block shows), so the full text is genuinely needed for the message. The duplication is the problem, not the content.
- Add
repeated string sql_text_pool to Operator, populated only on the root operator of a serialized native block.
- Add
optional int32 sql_text_idx to QueryContext; when set, sql_text is left empty.
- Intern as a post-pass in
CometNativeExec.convertBlock(), which is the point where the full operator tree for a block is in hand. Doing it there means none of the ~90 serde call sites change and no plan-scoped mutable state has to be threaded through exprToProto.
- Keep
sql_text working as a fallback when sql_text_idx is absent, so the paths that serialize nativeOp directly without going through convertBlock() (e.g. CometNativeWriteExec) keep working unchanged.
- On the native side, resolve
sql_text_idx against the pool and share one Arc<String> across all contexts that reference it, which also removes the per-expression String clone.
Error messages are unchanged; this is purely a wire-format deduplication.
Steps to reproduce
Plan any SQL query with Comet enabled, take CometNativeExec.serializedPlanOpt.plan.get, parse it with OperatorOuterClass.Operator.parseFrom, and walk the tree summing Expr.getQueryContext.getSerializedSize against the total byte count.
Expected behavior
The serialized plan should contain each distinct SQL text once, not once per expression.
Additional context
Found while doing a broader planner / serde / optimizer-rule performance audit: #5199
Describe the bug
QueryPlanSerdeattaches aQueryContextto every expression it serializes, andQueryContext.sql_textis the full SQL text of the query. The same string is therefore embedded once per expression in the serialized native plan.On a modest 2-table join + aggregate query (778-char SQL), 71 of the 90
Exprnodes in the plan carried a copy of the query text:TPC-DS queries are 1-3 KB of SQL, so the ratio gets worse, not better.
Where it comes from
QueryPlanSerde.exprToProtoInternalends with a.mapthat callsextractQueryContext(expr)for every converted expression;aggExprToProtodoes the same for aggregates.QueryContext.sql_textinnative/proto/src/proto/expr.protois declared as the full query text, andSQLQueryContext.sqlTextis what gets copied into it.Why it costs more than just bytes
Array[Byte]is captured byCometExecRDDand shipped in the task binary for the stage.CometExecRDD.computedoesOperator.parseFrom(serializedPlan)thenPlanDataInjector.injectPlanDatathenserializeOperator— per task. Measured at 0.153 ms vs 0.063 ms with the duplication removed. At 10k tasks that is ~1.5 s of JVM protobuf churn per stage, ~60% of it avoidable.PhysicalPlanner::create_exprandcreate_agg_exprdoctx_proto.sql_text.clone()into a freshStringand register it inQueryContextMap, per expression, per task. That is 71 identical heap strings per plan per task, held for the lifetime of the query. NoteQueryContext.sql_textis alreadyArc<String>internally, so the sharing was anticipated — it is just never realised because each proto entry decodes to its ownString.Proposed fix: intern the SQL text into a pool
Truncating to just the relevant fragment is not an option:
QueryContext::format_summaryprints the entiresql_text(that is what Spark's== SQL (line N, position M) ==block shows), so the full text is genuinely needed for the message. The duplication is the problem, not the content.repeated string sql_text_pooltoOperator, populated only on the root operator of a serialized native block.optional int32 sql_text_idxtoQueryContext; when set,sql_textis left empty.CometNativeExec.convertBlock(), which is the point where the full operator tree for a block is in hand. Doing it there means none of the ~90 serde call sites change and no plan-scoped mutable state has to be threaded throughexprToProto.sql_textworking as a fallback whensql_text_idxis absent, so the paths that serializenativeOpdirectly without going throughconvertBlock()(e.g.CometNativeWriteExec) keep working unchanged.sql_text_idxagainst the pool and share oneArc<String>across all contexts that reference it, which also removes the per-expressionStringclone.Error messages are unchanged; this is purely a wire-format deduplication.
Steps to reproduce
Plan any SQL query with Comet enabled, take
CometNativeExec.serializedPlanOpt.plan.get, parse it withOperatorOuterClass.Operator.parseFrom, and walk the tree summingExpr.getQueryContext.getSerializedSizeagainst the total byte count.Expected behavior
The serialized plan should contain each distinct SQL text once, not once per expression.
Additional context
Found while doing a broader planner / serde / optimizer-rule performance audit: #5199