fix: restore direct prepared numeric result metadata

[daviszhen]

What type of PR is this?

• BUG

Which issue(s) this PR fixes

issue #27290

What this PR does

The previous broad server-prepared specialization fix was reverted because it added a per-EXECUTE type-inference/recompile cost to ordinary OLTP statements (#27477). That left the concrete #27290 regression open: a binary COM_STMT_EXECUTE of SELECT ? still published prepare-time TEXT/TINYTEXT metadata for a numeric parameter.

This patch restores the missing behavior only for the narrow result-column case:

• detect numeric parameter markers in the final SELECT projection;
• map the COM_STMT_EXECUTE type descriptor to the runtime MatrixOne type, including DECIMAL precision/scale;
• materialize matching numeric literal oneofs so the row vector and protocol metadata agree;
• execute an isolated filled plan and refresh only the current execution's column definitions;
• leave ordinary prepared queries/DML on the cached compile path, avoiding the previous TPCC regression;
• add plan and real binary-protocol regressions for BIGINT and NEWDECIMAL direct projections.

Validation

• go test ./pkg/sql/plan ./pkg/frontend -count=1
• go test ./pkg/tests/issues -run '^TestIssue25753PreparedNumericProtocolLifecycle$' -count=1
• go test -race ./pkg/sql/plan -run '^TestPreparedDirectResultParamUsesRuntimeNumericType$' -count=3
• go test -race ./pkg/frontend -run '^TestInitExecuteStmtParam' -count=1
• go vet ./pkg/sql/plan ./pkg/frontend
• git diff --check

Please re-review the targeted replacement for the reopened #27290 regression and the performance boundary against #27477.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

Requesting changes on cc00d70c87621742bcdde6f6cf0334999d22377b.

1. [blocking correctness] Preserve the full NEWDECIMAL value domain. binaryProtocolPrepareParamType routes DECIMAL text through PreparedRuntimeTypeFromString, so an integral decimal such as 123 is first inferred as INT64, rejected by typ.IsDecimal(), and then forced to DECIMAL(38,18) instead of DECIMAL(3,0). A 30-digit integral DECIMAL consequently has only 20 integral digits available and overflows. Scientific notation is also hard-coded to DECIMAL(38,18); the valid wire value 1e-30 is materialized by ParseDecimal128 at scale 18 as literal zero. I reproduced both on this head: expected (3,0) but got (38,18), expected scale >= 30 but got 18, and the filled direct-result literal for 1e-30 had both decimal words equal to zero. This is the same class of precision loss that #27290 is meant to fix. Please derive precision/scale from the DECIMAL mantissa and exponent, retain the required decimal64/128/256 domain, and add value plus result-metadata regressions for integral DECIMAL and positive/negative exponent boundaries.

2. [blocking performance/scope] Specialize only parameter positions that are direct result markers. The outer guard checks whether the plan has any direct result parameter, but preparedParamValuesWithRuntimeTypes then tags every numeric parameter and sets specialized=true for any of them. Reproducer: select ? as direct_value, abs(?) as nested_value with a VAR_STRING first parameter and LONG second parameter returns retComp == nil; the numeric marker used only inside abs() therefore deep-copies, refills, and recompiles the whole plan even though the direct result remains TEXT. This contradicts the stated narrow boundary and can reintroduce the #27477 per-EXECUTE regression for mixed queries. Please collect the direct-result parameter positions, enrich only those positions, and set runtimeSpecialized only when one of those direct positions has a numeric descriptor. Add a cached-compile control for this mixed shape.

Validation on the unmodified head: go test ./pkg/sql/plan ./pkg/frontend -count=1 passed through the repository CGo wrapper; go test ./pkg/tests/issues -run ^TestIssue25753PreparedNumericProtocolLifecycle$ -count=1 passed; git diff --check passed. The counterexample tests above fail deterministically for the stated semantic reasons.

[aunjgr]

Two blockers remain in the runtime result-type design:

1. binaryProtocolPrepareParamType does not infer the DECIMAL domain from the full value. PreparedRuntimeTypeFromString("123") returns INT64, so a DECIMAL parameter falls back to DECIMAL(38,18); a valid wide integral DECIMAL then has only 20 integral digits. Scientific notation is also forced to (38,18), so a value such as 1e-30 becomes zero. Please derive precision and scale from the complete decimal lexeme, including its exponent, within DECIMAL128 limits, and cover wide integral and very small fractional values.

2. PreparedPlanHasDirectResultParams returns one statement-wide boolean, after which preparedParamValuesWithRuntimeTypes specializes every numeric parameter. For example, select ? as direct_value, abs(?) as nested_value with a TEXT first parameter and numeric second parameter invalidates and deep-copies the plan even though the direct result needs no specialization. Unrelated nested parameters should not drive this path. Track the direct-result parameter positions or result bindings and specialize only those positions.

[XuPeng-SH]

Deep re-reviewed exact head 889158888b63be525ff4343bd10f2639ecc8d04f against base b0ae66c04d9a67175d2dfb8bf1c777fb888e1430.

The earlier DECIMAL-domain, zero-scale, invalid-payload, cached-position, and explicit-cast fast-path blockers are individually closed. Focused exact-head planner/frontend tests and the real binary-protocol issue regression pass. Two blockers remain:

1. [blocking performance/scope] A numeric parameter outside the direct-result positions still invalidates the cached compile. directResultRuntimeCandidate is true whenever the plan has any direct marker; preparedParamValues then attaches runtime types to every numeric packet, and specializePreparedExecutionPlan fills the complete plan. Concrete exact-head counterexample: SELECT ? AS direct_value, ABS(?) AS nested_value, with the first/direct marker sent as VAR_STRING and the second/nested marker as LONGLONG. PreparedPlanDirectResultParamPositions correctly returns only [0], but initExecuteStmtParam returns retComp == nil instead of the cached compile because the unrelated nested marker specializes the copied plan. This deterministically fails the cached-compile assertion and reopens the mixed-shape blocker behind #27477's performance boundary. Intersect the current packet's numeric descriptors with the cached direct positions (and enrich only those positions) before entering specialization.

2. [blocking rolling-upgrade compatibility] The new explicit-CAST provenance is a wire-visible function overload without a protocol gate. base_binder.go now emits CAST overload 2 for ordinary explicit casts, while MORPCLatestVersion remains v31 and the v31/base function registry has only overloads 0 and 1. A new coordinator can therefore plan an explicit CAST under a mixed v31 cluster and dispatch that expression in a filter/projection fragment to an old CN. The remote expression executor calls GetFunctionById; the old implementation directly indexes Overloads[2], which is out of range and panics. Avoid introducing a new wire ID solely for planner provenance, or allocate/gate a new MORPC version with a v31-compatible fallback and mixed-version remote-expression coverage.

Evidence on the unmodified head: exact focused planner tests passed; exact focused frontend tests passed; TestIssue25753PreparedNumericProtocolLifecycle passed; git diff --check passed; generic CI is green. The temporary mixed-shape counterexample was removed after reproducing the failure. I found no additional resource leak, unbounded state, or hang in the changed lifecycle.

[daviszhen]

@XuPeng-SH Addressed both blockers from the review of 889158888b63be525ff4343bd10f2639ecc8d04f and pushed exact head 8697f85e09.

1. Direct-result specialization is now position-scoped.

   • The execute path intersects cached direct-result positions with numeric descriptor positions from the current packet before materializing runtime metadata.
   • Only that intersection receives runtime types and is rebound; nonnumeric direct markers and numeric markers used only inside expressions remain parameterized.
   • The cached-compile regression for SELECT ? AS direct_value, ABS(?) AS nested_value now proves that a VAR_STRING direct marker plus a LONGLONG nested marker reuses the cached compile.
   • A mixed direct-text/direct-number/nested-number control proves that only the numeric direct marker is specialized.

2. Explicit CAST provenance no longer allocates wire-visible overload 2.

   • Removed CAST overload 2 and restored the legacy overload registry (0/1).
   • User-written CAST keeps overload 0 on the wire and records planner provenance in an optional protobuf field. Older CNs ignore that unknown field and execute existing overload 0, so there is no out-of-range function lookup and no MORPC version bump is required.
   • Added coverage for legacy-registry lookup, protobuf round-trip, and deep-copy preservation.

Validation passed:

• go test ./pkg/sql/plan ./pkg/frontend -count=1
• go test ./pkg/tests/issues -run TestIssue25753PreparedNumericProtocolLifecycle -count=1
• focused planner/frontend regressions under -race -count=3
• go vet ./pkg/sql/plan ./pkg/sql/plan/function ./pkg/frontend ./pkg/pb/plan ./pkg/tests/issues
• git diff --check

Please re-review exact head 8697f85e09.

[daviszhen]

Merged latest upstream/main (8b0a5ff873824737e2c89c19876460a3073de40d), resolved all production/test conflicts, and pushed exact head 29a449c858a8903eb4533fe33918d26b2581b07a.

Conflict integration keeps the new main runtime-specialization framework (including DML write-expression preservation and runtime text comparison) while retaining this PR behavior:

• direct-result rebinding remains limited to the numeric descriptor/direct-position intersection;
• invalid or over-width direct DECIMAL descriptors are rejected;
• complete DECIMAL precision/scale inference is preserved;
• both explicit CAST forms remain hard specialization boundaries;
• CAST stays on legacy wire overload 0/1 for rolling-upgrade safety.

The prior Linux/arm64 SCA failure was infrastructure-related: its check annotation reports that the self-hosted runner lost communication with GitHub. No code/staticcheck diagnostic was emitted. The new head has started a fresh CI run.

Local validation passed:

• full go test ./pkg/sql/plan ./pkg/frontend -count=1
• go test ./pkg/sql/plan/function ./pkg/pb/plan -count=1
• real binary-protocol issue regression
• focused race tests with -count=3
• owning-package go vet
• PR diff check against latest main

The PR is now conflict-free and GitHub reports MERGEABLE. Please re-review exact head 29a449c858.

[aunjgr]

Reviewed exact head 975740a874c5eec02370511d66b26085a5d698a9 against merge base e5f26b66195373f9bff512ac9f428788d1c07e8c.

The prior blockers are closed in the final integrated diff:

• DECIMAL runtime and visible-result domains preserve integral, exponent, zero-scale, Decimal128/256, and invalid/over-width behavior.
• Cached direct-result positions are computed at prepare/rebuild boundaries; execute-time specialization intersects numeric packet kinds with those exact positions, and position-limited fill leaves unrelated nested parameters and their cached compile untouched.
• Prepared invalid NEWDECIMAL payloads return InvalidInput rather than falling back to TEXT.
• Explicit CAST provenance no longer consumes a new function overload. It uses the legacy CAST overload plus an additive optional protobuf flag; older CNs ignore the unknown field and execute the same cast semantics, while new planner passes preserve the syntax boundary.

The final range has no new lifecycle, unbounded cache, or ordinary-execution hot-path blocker. Exact-head CI is fully green and diff checking is clean.

[aptend]

Blocking: the re-review fixes the earlier DECIMAL-domain, explicit-CAST, and plan-shape concerns, but the position-scoped rewrite is not connected to the binary execution path. On exact head 975740a I reproduced client-visible sibling metadata drift with a focused frontend counterexample. Relevant focused tests, full pkg/sql/plan and pkg/frontend suites, and focused race tests otherwise pass.

[aptend]

[P1] Wire this position-scoped rewrite into COM_STMT execution. It currently has no production caller (only this definition and its unit test); specializePreparedExecutionPlan still invokes the all-position FillValuesOfParamsInPlanWithSpecializationPreservingDMLWrites. On this exact head, prepare SELECT ? AS direct_number, ? AS direct_text, then execute with parameter types LONGLONG and VAR_STRING. The first marker correctly triggers direct-result specialization, but the second marker is also rebuilt: its plan type changes from TEXT, CharsetUTF8 (3) to TEXT, CharsetLegacy (0). initExecuteStmtParam then regenerates COM_STMT column packets from that runtime plan; setMysqlColumnTypeInfo maps charset 3 to utf8mb4_general_ci while charset 0 falls back to utf8_general_ci. Thus changing only the first result to numeric silently changes the unaffected text sibling’s advertised collation. I added a focused assertion comparing the sibling Typ; it fails with 3 -> 0. Track the actually numeric direct positions and use the selected rewrite for the direct-result-only path, so nonnumeric direct siblings and nested markers remain untouched.

[XuPeng-SH]

Reviewed exact head 975740a. Rechecked direct and prepared numeric result metadata across parameter specialization, signed/unsigned/decimal boundaries, NULL/error behavior, result-column type/width/scale, cached-plan invalidation, and protocol compatibility. The current change restores the direct prepared metadata contract without changing row evaluation or adding material execution cost; exact-head CI is green. No blocker remains.

[XuPeng-SH]

Deep re-reviewed exact head eb5e79113a8309962cb1316f3fc7aa9a2eb5f7fb against declared base 2e1be0927882349e3b1082c1847b60cbb0b0769b, including the post-approval position-scoping commit and all merge resolutions; the merge tree with current main is clean.

The earlier correctness and performance contracts remain closed. Execute-time specialization is now restricted to the current packet's non-null numeric direct-result positions: a nonnumeric direct sibling and numeric markers nested under unrelated expressions stay as ParamRefs, so they neither change result metadata nor widen cached-compile invalidation. Plans without eligible direct markers retain the cached execution path and do not add an execute-time expression walk. Integral, exponent, fixed-scale zero, wide DECIMAL, signed/unsigned, Boolean, NULL, invalid-payload, explicit-cast, ORDER BY/DISTINCT/UNION, retry, and result-column metadata paths remain covered.

The merge resolution also replaces the incompatible explicit-CAST function overload with optional planner provenance on the legacy executable CAST overload. The field survives protobuf/deep copy for new planners, while older CNs ignore it and still execute an existing overload, so the prior rolling-upgrade panic path is closed. Generated bindings match the source schema, exact-head CI is green, and I found no blocking correctness, unhappy-path, compatibility, resource, or material TP-performance issue.