You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "csun5285 (via GitHub)" <gi...@apache.org> on 2026/04/28 08:30:55 UTC
[PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
csun5285 opened a new pull request, #62905:
URL: https://github.com/apache/doris/pull/62905
Fix:
- InsertIntoValuesAnalyzer: include CheckCast.INSTANCE in INSERT_JOBS and BATCH_INSERT_JOBS so every remaining Cast is type-checked.
- VariantType.acceptsType / isAssignableFrom: align with DecimalV3Type behaviour - equality only when configurations match.
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 closed pull request #62905: [fix](variant) Reject Variant->Variant cast with mismatched parameters
URL: https://github.com/apache/doris/pull/62905
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4394044264
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "924060929 (via GitHub)" <gi...@apache.org>.
924060929 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4418307929
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4335526818
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4418258317
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "airborne12 (via GitHub)" <gi...@apache.org>.
airborne12 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4404912631
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "924060929 (via GitHub)" <gi...@apache.org>.
924060929 merged PR #62905:
URL: https://github.com/apache/doris/pull/62905
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "924060929 (via GitHub)" <gi...@apache.org>.
924060929 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4408084315
## Simpler fix: validate in `castUnbound` instead of adding rewrite rules
The bug is real — mismatched `variant<config_A>` → `variant<config_B>` casts slip through FE and crash BE. But the fix can be much simpler and avoid adding overhead to the INSERT VALUES fast path.
### Root cause
`TypeCoercionUtils.castUnbound()` has two branches:
- **Literal**: calls `castIfNotSameType()` → calls `checkCanCastTo()` ✅
- **Non-Literal**: calls `unSafeCast()` → skips validation entirely ❌
For `cast('json' as variant<config_A>)`, the parser produces a `Cast` node whose `getDataType()` returns the fully-resolved `VariantType` (with all properties from SQL text or session variables) at **parse time** — no analysis/binding needed. So we can validate it right there.
### Proposed fix (one line)
```java
public static Expression castUnbound(Expression expression, DataType targetType) {
if (expression instanceof Literal) {
return TypeCoercionUtils.castIfNotSameType(expression, targetType);
} else {
+ try {
+ checkCanCastTo(expression.getDataType(), targetType);
+ } catch (UnboundException e) {
+ // Unbound expressions (e.g. function calls) — validated after analysis
+ }
return TypeCoercionUtils.unSafeCast(expression, targetType);
}
}
```
### Why this works
`Cast.getDataType()` returns `targetType` — it's declarative (set at construction), not derived from binding. So at `castUnbound` time:
- `cast('json' as variant<A>)` → `expression.getDataType()` = `variant<A>` ✅ known
- `bitmap_hash('x')` → `UnboundFunction.getDataType()` throws `UnboundException` → caught and skipped
I verified this by building a test cluster from master, adding `checkCanCastTo` to `castUnbound`, and running the PR's test cases:
| Test case | Result |
|---|---|
| `cast(json as variant<mismatch>)` single row | **Rejected at FE** ✅ |
| `cast(json as variant<mismatch>)` multi-row VALUES | **Rejected at FE** ✅ |
| `cast(json as variant<mismatch>)` in txn (BEGIN) | **Rejected at FE** ✅ |
| Normal `insert into t values ('json')` | Works ✅ |
| `cast(json as variant)` matching config | Works ✅ |
| Multi-row normal VALUES | Works ✅ |
### Why the current PR approach is heavier than needed
The current PR:
1. Adds `CheckCast.INSTANCE` to `INSERT_JOBS` and `BATCH_INSERT_JOBS` rewrite pipelines
2. Adds `LogicalSetOperationRewrite` to `RewriteInsertIntoExpressions` (to visit `LogicalUnion.constantExprsList`)
3. Changes `LogicalSetOperationRewrite` visibility from `private` to `public`
This adds an expression tree traversal to the INSERT VALUES fast path — which exists specifically to skip full rewrite/optimize. The `LogicalSetOperationRewrite` visits every expression in every row of a multi-row INSERT, costing O(N×C) for N rows × C columns.
The `castUnbound` fix catches the same bug at the source, before the expression tree is even constructed, at O(1) per value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4394044120
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62905:
URL: https://github.com/apache/doris/pull/62905#discussion_r3216956893
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -476,10 +477,27 @@ public static Expression castIfNotSameType(Expression input, DataType targetType
}
}
+ /**
+ * Wrap {@code expression} in a cast to {@code targetType} when the source type can
+ * already be resolved (Literal, or any expression whose {@code getDataType()} does
+ * not throw {@link UnboundException}). For Literals, defers to
+ * {@link #castIfNotSameType}; for other resolvable expressions, validates the cast
+ * via {@link #checkCanCastTo} before constructing it. Used by INSERT VALUES so that
+ * illegal casts (e.g. {@code variant<config_A>} → {@code variant<config_B>}) are
+ * rejected at parse time instead of slipping past analysis and crashing BE.
+ * Truly unbound expressions are wrapped without validation; the normal analysis
+ * pipeline's CheckCast pass will validate them after binding.
+ */
public static Expression castUnbound(Expression expression, DataType targetType) {
if (expression instanceof Literal) {
return TypeCoercionUtils.castIfNotSameType(expression, targetType);
} else {
+ try {
+ checkCanCastTo(expression.getDataType(), targetType);
Review Comment:
This validation still lets some mismatched Variant-to-Variant casts through because `checkCanCastTo` calls `CheckCast.check`, and the Variant-to-Variant branch there only does `originalType.equals(targetType)`. `VariantType.equals` currently compares `variantMaxSubcolumnsCount`, `enableTypedPathsToSparse`, `enableVariantDocMode`, `variantDocMaterializationMinRows`, and fields, but it ignores writer-relevant properties such as `variantDocShardCount`, `variantSparseHashShardCount`, `variantMaxSparseColumnStatisticsSize`, and `enableNestedGroup` (while `hashCode` includes most of them). A source like `cast(... as variant<properties("variant_enable_doc_mode"="true", "variant_doc_materialization_min_rows"="8", "variant_doc_hash_shard_count"="999")>)` inserted into the test table with doc hash shard count 7 will therefore pass this new FE check even though the PR states BE has no real Variant-to-Variant conversion for mismatched configs. Please make the Variant-to-Variant compatibility
check compare all persisted/writer-affecting Variant properties, and add regression coverage for at least one of the currently ignored properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4417577574
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4418261781
# FE Regression Coverage Report
Increment line coverage ` 1.73% (3/173)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62905_a49cfd275a4b259f898a7426a057bdb484db0a75_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62905_a49cfd275a4b259f898a7426a057bdb484db0a75_merge_fe/report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4336055447
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4343646927
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4333636366
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4404619124
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4384617144
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4384627121
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4333635450
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4349213475
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62905:
URL: https://github.com/apache/doris/pull/62905#discussion_r3198903739
##########
regression-test/suites/variant_p0/doc_mode/test_variant_cast_strict.groovy:
##########
@@ -0,0 +1,106 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// INSERT VALUES with explicit cast(<json> as variant) into a doc-mode
+// variant column previously aborted BE in MutableBlock::merge_impl.
+// The root cause: FE allowed implicit Variant->Variant cast even when
+// configurations (max_subcolumns_count, enable_doc_mode, ...) differ,
+// and BE has no real Variant->Variant conversion.
+//
+// Fix: FE rejects Variant->Variant cast when configurations differ.
+// User-visible behavior:
+// * BAD : cast('<json>' as variant) with mismatched config -> AnalysisException
+// * GOOD: '<json>' -> auto-coerce to target
+// * GOOD: cast('<json>' as variant<...matching...>) -> direct
+suite("test_variant_cast_strict", "p0") {
+ // Use session variables to set variant defaults (column-level properties
+ // forbid setting max_subcolumns_count and enable_doc_mode together).
+ sql """ set default_variant_enable_doc_mode = true """
+ sql """ set default_variant_max_subcolumns_count = 37 """
+ sql """ set default_variant_doc_materialization_min_rows = 8 """
+ sql """ set default_variant_doc_hash_shard_count = 7 """
+
+ def t = "variant_cast_strict"
+ sql """ DROP TABLE IF EXISTS ${t} """
+ sql """
+ CREATE TABLE IF NOT EXISTS ${t} (
+ id bigint,
+ v variant
+ )
+ DUPLICATE KEY(`id`)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES("replication_num" = "1", "disable_auto_compaction" = "true");
+ """
+
+ def jsonValue = '{"anchors":{"common_int":150025,"phase_marker":"phase_a","present":true,"row_id":15001},"dynamic":{"path_00000":15001000,"path_00001":15001001},"parent":{"child":{"name":"phase_a_15001"}},"phase_a_small":{"leaf":15001}}'
+
+ // ---- Case 1: BAD path — implicit Variant->Variant with mismatched config should be
+ // rejected by FE. Before fix: BE aborts in MutableBlock::merge_impl. After fix: FE
+ // throws AnalysisException; BE never receives the plan.
+ // We force a config mismatch on `variant_doc_materialization_min_rows` (target=8 from
+ // session, source=999 here).
+ test {
+ sql """ insert into ${t} values (15001, cast('${jsonValue}' as variant<properties(
+ "variant_enable_doc_mode" = "true",
+ "variant_doc_materialization_min_rows" = "999",
+ "variant_doc_hash_shard_count" = "7"
+ )>)); """
+ exception "cast"
+ }
+
+ // ---- Case 2: GOOD — drop the cast, let FE coerce String -> target Variant directly.
+ sql """ insert into ${t} values (15002, '${jsonValue}'); """
+ qt_case2 """ select id, cast(v['anchors']['row_id'] as bigint) from ${t} where id = 15002; """
+
+ // ---- Case 3: GOOD — explicit cast with matching parameters (from same session).
+ // Bare `as variant` reads session-default config and matches target exactly.
+ sql """ insert into ${t} values (15003, cast('${jsonValue}' as variant)); """
+ qt_case3 """ select id, cast(v['anchors']['row_id'] as bigint) from ${t} where id = 15003; """
+
+ // ---- Case 4: cross-table — different variant configs need an explicit JSONB hop.
+ def t_src = "variant_cast_strict_src"
+ // Create source table with NO doc-mode by clearing session vars first, then restore.
+ sql """ set default_variant_enable_doc_mode = false """
+ sql """ set default_variant_max_subcolumns_count = 0 """
+ sql """ DROP TABLE IF EXISTS ${t_src} """
+ sql """
+ CREATE TABLE IF NOT EXISTS ${t_src} (
+ id bigint,
+ v variant
+ )
+ DUPLICATE KEY(`id`)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES("replication_num" = "1", "disable_auto_compaction" = "true");
+ """
+ sql """ insert into ${t_src} values (15004, '${jsonValue}'); """
+ // Restore session vars so target's column-level config keeps matching.
+ sql """ set default_variant_enable_doc_mode = true """
+ sql """ set default_variant_max_subcolumns_count = 37 """
+
+ // 4a: direct copy is rejected (configs differ).
+ test {
+ sql """ insert into ${t} select id, v from ${t_src}; """
+ exception "cast"
+ }
+
+ // 4b: routing through JSONB works.
+ sql """ insert into ${t} select id, cast(cast(v as JSONB) as variant) from ${t_src}; """
+ qt_case4b """ select id, cast(v['anchors']['row_id'] as bigint) from ${t} where id = 15004; """
+
+ sql """ DROP TABLE IF EXISTS ${t} """
+ sql """ DROP TABLE IF EXISTS ${t_src} """
Review Comment:
The regression-test convention in this repository is to drop tables before use, not after completion, so the final state is preserved for debugging failed runs. This suite already drops both tables before creating them, so please remove the cleanup drops at the end.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoValuesAnalyzer.java:
##########
@@ -79,7 +81,8 @@ public class InsertIntoValuesAnalyzer extends AbstractBatchJobExecutor {
new RewriteInsertIntoExpressions(ExpressionRewrite.bottomUp(
ConvertAggStateCast.INSTANCE,
- FoldConstantRuleOnFE.PATTERN_MATCH_INSTANCE
Review Comment:
This does not actually validate casts on the batch insert path after `PushProjectIntoUnion`. In `BATCH_INSERT_JOBS`, `BindSink` creates the target-column casts in a `LogicalProject`, then `PushProjectIntoUnion` pushes those projected expressions into `LogicalUnion.constantExprsList` and removes the project. However this custom `RewriteInsertIntoExpressions` only builds `ProjectExpressionRewrite` and `OneRowRelationExpressionRewrite`, unlike the normal `ExpressionRewrite` which has `LogicalSetOperationRewrite` to visit `LogicalUnion` constant expressions. As a result, any casts that were pushed into the union constants bypass the newly added `CheckCast.INSTANCE`, so batch INSERT VALUES still has the same unchecked-cast hole this PR is meant to close. Please either run `CheckCast` before pushing the project into the union, or extend this insert rewrite to also validate/rewrite the union constant expressions, and add a regression case that exercises the batch path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4417682592
<details>
<summary>TPC-DS: <b>Total hot run time: 171124 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit a49cfd275a4b259f898a7426a057bdb484db0a75, data reload: false
query5 4297 655 523 523
query6 326 227 204 204
query7 4269 545 299 299
query8 339 240 227 227
query9 8864 4062 4051 4051
query10 451 343 307 307
query11 5818 2445 2210 2210
query12 190 136 130 130
query13 1323 621 448 448
query14 6286 5444 5087 5087
query14_1 4435 4421 4332 4332
query15 209 206 180 180
query16 990 485 422 422
query17 1134 755 601 601
query18 2765 472 351 351
query19 214 198 164 164
query20 141 130 129 129
query21 224 134 124 124
query22 13546 13947 14708 13947
query23 17421 16600 16172 16172
query23_1 16343 16320 16313 16313
query24 7439 1747 1341 1341
query24_1 1353 1355 1375 1355
query25 598 521 453 453
query26 1302 325 176 176
query27 2685 623 347 347
query28 4464 1991 1988 1988
query29 1044 650 547 547
query30 305 237 201 201
query31 1138 1082 948 948
query32 83 74 77 74
query33 551 367 300 300
query34 1197 1129 652 652
query35 771 799 679 679
query36 1352 1355 1181 1181
query37 156 112 91 91
query38 3221 3104 3071 3071
query39 926 913 886 886
query39_1 861 858 893 858
query40 231 155 143 143
query41 69 66 66 66
query42 113 112 110 110
query43 322 335 289 289
query44
query45 209 203 190 190
query46 1104 1221 731 731
query47 2250 2226 2190 2190
query48 405 428 315 315
query49 639 575 427 427
query50 701 279 216 216
query51 4308 4269 4223 4223
query52 102 104 96 96
query53 258 289 202 202
query54 323 283 263 263
query55 92 88 83 83
query56 313 301 303 301
query57 1394 1383 1282 1282
query58 298 283 275 275
query59 1523 1599 1399 1399
query60 349 336 323 323
query61 158 155 152 152
query62 673 615 574 574
query63 235 197 203 197
query64 2370 823 704 704
query65
query66 1679 507 393 393
query67 29929 29915 29764 29764
query68
query69 458 332 309 309
query70 1038 986 997 986
query71 309 275 275 275
query72 2982 2668 2435 2435
query73 850 760 431 431
query74 5091 4898 4742 4742
query75 2792 2653 2327 2327
query76 2286 1128 796 796
query77 454 429 353 353
query78 13011 13014 12473 12473
query79 1511 977 728 728
query80 1373 579 511 511
query81 509 289 236 236
query82 1324 159 128 128
query83 338 273 256 256
query84 264 144 110 110
query85 922 519 459 459
query86 458 321 310 310
query87 3423 3332 3236 3236
query88 3608 2664 2675 2664
query89 448 377 339 339
query90 1914 185 186 185
query91 178 172 144 144
query92 77 75 73 73
query93 969 953 569 569
query94 726 344 296 296
query95 668 456 347 347
query96 993 826 339 339
query97 2672 2700 2588 2588
query98 237 232 226 226
query99 1109 1114 972 972
Total cold run time: 254699 ms
Total hot run time: 171124 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4417940052
# FE UT Coverage Report
Increment line coverage ` 0.00% (0/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62905_a49cfd275a4b259f898a7426a057bdb484db0a75/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62905_a49cfd275a4b259f898a7426a057bdb484db0a75/fe_report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4341273599
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4340503405
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](variant) Reject Variant->Variant cast with mismatched parameters [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4336577961
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4417637171
<details>
<summary>TPC-H: <b>Total hot run time: 29578 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit a49cfd275a4b259f898a7426a057bdb484db0a75, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17591 3866 3982 3866
q2 q3 10703 883 601 601
q4 4666 465 347 347
q5 7459 1330 1155 1155
q6 186 168 139 139
q7 906 954 764 764
q8 9320 1449 1287 1287
q9 5633 5399 5331 5331
q10 6247 2084 1810 1810
q11 466 273 261 261
q12 635 422 296 296
q13 18068 3285 2755 2755
q14 294 284 268 268
q15 q16 900 867 787 787
q17 928 1036 704 704
q18 6483 5609 5594 5594
q19 1359 1236 1033 1033
q20 517 397 262 262
q21 5195 2404 1985 1985
q22 495 393 333 333
Total cold run time: 98051 ms
Total hot run time: 29578 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4706 4772 4778 4772
q2 q3 4710 4814 4242 4242
q4 2133 2177 1434 1434
q5 5023 4981 5291 4981
q6 203 171 136 136
q7 2097 1790 1661 1661
q8 3388 3119 3178 3119
q9 8410 8349 8436 8349
q10 4547 4535 4263 4263
q11 604 417 404 404
q12 699 754 530 530
q13 3273 3593 2842 2842
q14 307 306 342 306
q15 q16 799 787 744 744
q17 1324 1300 1238 1238
q18 7978 7154 7132 7132
q19 1150 1159 1145 1145
q20 2247 2234 1964 1964
q21 6183 5503 4896 4896
q22 537 500 401 401
Total cold run time: 60318 ms
Total hot run time: 54559 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) Run CheckCast on INSERT VALUES to reject illegal casts [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on PR #62905:
URL: https://github.com/apache/doris/pull/62905#issuecomment-4394023244
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org