github-actions[bot] commented on code in PR #68016:
URL: https://github.com/apache/doris/pull/68016#discussion_r4014966973
##########
be/src/storage/segment/variant/v2/variant_path_builder.cpp:
##########
@@ -376,6 +396,10 @@ DataTypePtr path_least_common_type(const DataTypePtr&
left, const DataTypePtr& r
}
return jsonb_type();
}
+ if ((left_primitive == TYPE_BOOLEAN &&
is_non_boolean_pure_numeric_type(right_primitive)) ||
Review Comment:
[P1] Apply this rule when compaction merges Variant segment schemas
This special case only runs when both values meet in the same
`VariantPathBuilder`. With separate inserts, one segment can publish `k` as
`BOOLEAN` and another as `BIGINT`. Default-enabled Variant subcolumn compaction
gathers both physical types, and `get_compaction_subcolumns_from_data_types()`
still resolves them with the generic `get_least_supertype_jsonb()` to `BIGINT`;
the merger's `SegmentIterator` then casts the BOOL leaf to the target BIGINT,
rewriting true/false as 1/0 in the compacted output. Please share the
Variant-specific BOOL/number rule with the production compaction schema-union
helpers (including nested and nested-group regular subcolumns) and add a
separate-segment regression that checks display/GROUP BY before and after
forced compaction. The single-INSERT test never crosses this boundary.
##########
regression-test/suites/variant_p0/test_variant_bool_numeric_widening.groovy:
##########
@@ -0,0 +1,110 @@
+// 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.
+
+// Regression coverage for a Variant V2 storage bug: within a single segment,
when a path's first
+// value is a JSON bool and a plain number arrives afterwards,
get_numeric_type() (BE
+// core/data_type/get_least_supertype.cpp) counts TYPE_BOOLEAN as an 8-bit
unsigned integer, so the
+// path's least-common-type computation folded BOOL and the numeric type into
a single numeric
+// storage type instead of falling back to JSONB. promote() then cast the
already-written `true`
+// value to that numeric type, turning it into `1` in storage -- a real
Variant/JSON semantics
+// violation (true must never compare or group equal to 1). The opposite order
(a number first, then
+// a bool) already fell back to JSONB and stayed correct; the "rev" path below
is that same-order
+// control, which must keep passing unchanged.
+//
+// Note on the queries below: Doris rejects any direct comparison predicate on
a raw
+// VARIANT/JSON-typed expression ("... does not support ordering/comparison,
CAST to a concrete type
+// first"), and CAST(... AS STRING)/CAST(... AS BOOLEAN) both render a SQL
boolean as "1"/"0" (normal,
+// unrelated SQL cast semantics), which would mask this exact bug instead of
exposing it. So the
+// checks below use the value's own canonical JSON text (the plain, uncast
SELECT output) and GROUP
+// BY on the raw variant sub-path (which only needs hashing/equality, not
ordering) to observe the
+// real stored value instead.
+suite("test_variant_bool_numeric_widening", "p0") {
+ def variantV2Function = "parse_to_variant"
+ def table_name = "test_variant_bool_numeric_widening"
Review Comment:
[P3] Hardcode this single test table name
The repository test contract requires ordinary single-table regression cases
to hardcode the table name instead of introducing a `def tableName`-style
variable. This suite has only one fixed table, so please remove `table_name`
and use `test_variant_bool_numeric_widening` directly in the DROP, CREATE,
INSERT, and query statements.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
