Skip to content

fix load data report 'unsupported cast from json to JSON'#23825

Merged
mergify[bot] merged 3 commits intomatrixorigin:mainfrom
robll-v1:issue/23822
Mar 11, 2026
Merged

fix load data report 'unsupported cast from json to JSON'#23825
mergify[bot] merged 3 commits intomatrixorigin:mainfrom
robll-v1:issue/23822

Conversation

@robll-v1
Copy link
Collaborator

@robll-v1 robll-v1 commented Mar 9, 2026

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #23822

What this PR does / why we need it:

Fix LOAD DATA ... PARALLEL 'TRUE' failure on tables with JSON columns and irregular indexes (for example IVFFLAT).

This change adds a safe identity cast path for JSON -> JSON, which prevents parallel load from failing with:

internal error: unsupported cast from json to JSON
Root Cause
For LOAD DATA on tables with irregular indexes, the planner may fall back to the legacy load path.
In the parallel branch, that path introduces an extra cast layer. For JSON columns, execution could reach the generic jsonToOthers() cast path.

Before this change, jsonToOthers() did not handle JSON -> JSON, so parallel load could fail even though the source and target types were both JSON.

Fix
Add an explicit identity cast branch for JSON -> JSON in pkg/sql/plan/function/func_cast.go
Copy JSON values safely via GetStrValue()/AppendBytes() instead of using a generic fixed-type duplication path
Add a regression unit test for JSON -> JSON cast in pkg/sql/plan/function/func_cast_test.go
Impact
This fixes parallel LOAD DATA for scenarios like:

JSON column present
IVFFLAT index present
LOAD DATA URL S3OPTION ... PARALLEL 'TRUE'
The fix is intentionally minimal and does not change the broader legacy load path behavior.

Files Changed
pkg/sql/plan/function/func_cast.go
pkg/sql/plan/function/func_cast_test.go
Validation
Unit Test
go test ./pkg/sql/plan/function -run 'TestCastJsonTo(Json|Numeric)' -count=1
Build
make build
Runtime Verification
Verified with a real LOAD DATA URL S3OPTION ... PARALLEL 'TRUE' flow against local MinIO.

Scenario:

table with JSON column
VECF64 column
IVFFLAT index
large CSV file (parallel path actually triggered)
Result:

before fix: internal error: unsupported cast from json to JSON
after fix: parallel load succeeded

Notes
This PR is a minimal functional fix to unblock the reported issue.
If needed, the legacy parallel LOAD DATA cast path can be further cleaned up in a follow-up change.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@qodo-code-review
Copy link

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Fix JSON to JSON cast in parallel LOAD DATA operations

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add identity cast path for JSON to JSON conversion
• Prevent parallel LOAD DATA failures on tables with JSON columns
• Use safe value copying via GetStrValue()/AppendBytes() methods
• Add regression test for JSON to JSON cast operation
Diagram
flowchart LR
  A["LOAD DATA PARALLEL on JSON column"] -->|encounters irregular index| B["Legacy load path with cast layer"]
  B -->|JSON to JSON cast| C["jsonToOthers function"]
  C -->|before fix| D["❌ unsupported cast error"]
  C -->|after fix| E["✓ Identity cast with safe copy"]
  E --> F["Parallel load succeeds"]
Loading

Grey Divider

File Changes

1. pkg/sql/plan/function/func_cast.go 🐞 Bug fix +9/-0

Add JSON to JSON identity cast handler

• Add explicit case handler for types.T_json in jsonToOthers() function
• Implement identity cast using GetStrValue() and AppendBytes() for safe JSON value copying
• Handle null values correctly during the cast operation

pkg/sql/plan/function/func_cast.go


2. pkg/sql/plan/function/func_cast_test.go 🧪 Tests +16/-0

Add regression test for JSON to JSON cast

• Add new test function TestCastJsonToJson() for JSON to JSON casting
• Test with multiple JSON values including objects, arrays, and null
• Verify cast preserves original encoded JSON data

pkg/sql/plan/function/func_cast_test.go


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Mar 9, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Planner rejects JSON→JSON 🐞 Bug ✓ Correctness
Description
jsonToOthers() now implements JSON→JSON at execution time, but CAST overload resolution still
rejects (json,json) because supportedTypeCast for T_json omits T_json. As a result, binder-generated
CAST expressions can fail in GetFunctionByName("cast", [json,json]) even though the execution path
now supports it.
Code

pkg/sql/plan/function/func_cast.go[R1834-1842]

+	case types.T_json:
+		rs := vector.MustFunctionResult[types.Varlena](result)
+		for i := uint64(0); i < uint64(length); i++ {
+			v, null := source.GetStrValue(i)
+			if err := rs.AppendBytes(v, null); err != nil {
+				return err
+			}
+		}
+		return nil
Evidence
Execution support was added in jsonToOthers() for toType.Oid==T_json, but the CAST operator’s type
check consults IfTypeCastSupported(), which only returns true when the target OID is present in
supportedTypeCast[source]. supportedTypeCast[types.T_json] does not include types.T_json, so
IfTypeCastSupported(T_json,T_json) is false; the binder always constructs CAST expressions by
calling function.GetFunctionByName("cast", argsType), so planning/binding a JSON→JSON cast can fail
before reaching the new runtime path. The new unit test calls NewCast directly (bypassing
binder/type-check), so it doesn’t cover this mismatch.

pkg/sql/plan/function/func_cast.go[1830-1846]
pkg/sql/plan/function/func_cast.go[363-369]
pkg/sql/plan/function/func_cast.go[411-420]
pkg/sql/plan/function/list_operator.go[2549-2560]
pkg/sql/plan/base_binder.go[135-146]
pkg/sql/plan/base_binder.go[2401-2410]
pkg/sql/plan/function/func_testcase.go[141-142]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`jsonToOthers()` now supports JSON→JSON casts at execution time, but CAST overload resolution still rejects `(T_json -&gt; T_json)` because `supportedTypeCast[types.T_json]` does not include `types.T_json`. This can cause explicit `CAST(json_expr AS JSON)` (and any binder-inserted JSON→JSON cast) to fail during `GetFunctionByName(&quot;cast&quot;, ...)` even though runtime can execute it.

### Issue Context
- CAST operator resolution uses `IfTypeCastSupported(inputs[0].Oid, inputs[1].Oid)`.
- `IfTypeCastSupported` only returns true when the target OID is present in `supportedTypeCast[source]`.
- Unit tests added in this PR call `NewCast` directly and do not validate overload resolution/binding.

### Fix Focus Areas
- pkg/sql/plan/function/func_cast.go[363-369]
- pkg/sql/plan/function/func_cast.go[411-420]
- pkg/sql/plan/function/list_operator.go[2549-2560]
- pkg/sql/plan/function/func_cast_test.go[2491-2505]

### Suggested fix
1. Update `supportedTypeCast` for `types.T_json` to include `types.T_json` (identity cast).
2. Add a regression test that ensures overload resolution succeeds for JSON→JSON, e.g. calling `function.GetFunctionByName(ctx, &quot;cast&quot;, []types.Type{types.T_json.ToType(), types.T_json.ToType()})` and asserting it returns successfully (or asserting `IfTypeCastSupported(types.T_json, types.T_json)` is true).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label Mar 9, 2026
@mergify mergify bot added the kind/bug Something isn't working label Mar 9, 2026
@mergify mergify bot added the queued label Mar 11, 2026
@mergify
Copy link
Contributor

mergify bot commented Mar 11, 2026

Merge Queue Status

  • Entered queue2026-03-11 10:28 UTC · Rule: main
  • 🚫 Left the queue2026-03-11 10:28 UTC · at 214572f28f7cfc59c1c13a9f1ad4bf9ada2cd082

This pull request spent 5 seconds in the queue, with no time running CI.

Reason

The pull request can't be updated

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/settings-print.yaml without workflows permission

Hint

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

@mergify mergify bot added dequeued and removed queued labels Mar 11, 2026
@mergify
Copy link
Contributor

mergify bot commented Mar 11, 2026

Merge Queue Status

  • Entered queue2026-03-11 11:23 UTC · Rule: main
  • Checks passed · in-place
  • Merged2026-03-11 11:23 UTC · at 82057606ec46eec646cba81a31590d31db6e3dc8

This pull request spent 11 seconds in the queue, with no time running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-success = Matrixone Utils CI / Coverage

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

Labels

kind/bug Something isn't working size/S Denotes a PR that changes [10,99] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants