github-actions[bot] commented on code in PR #69040:
URL: https://github.com/apache/doris/pull/69040#discussion_r4298702482
##########
be/src/exprs/function/cast/function_cast.cpp:
##########
@@ -197,12 +198,25 @@ WrapperType prepare_remove_nullable(FunctionContext*
context, const DataTypePtr&
block.insert(block.get_by_position(arguments[0]));
}
- const NullMap::value_type* arg_null_map = nullptr;
+ // FILE validation must also skip values hidden by an enclosing
NULL container.
+ const NullMap::value_type* arg_null_map =
(contains_file_type(from_type_not_nullable) ||
+
contains_file_type(to_type_not_nullable))
+ ? null_map
+ : nullptr;
+ NullMap combined_null_map;
if (source_info.is_nullable) {
arg_null_map = block.get_by_position(arguments[0])
.get_nullable_null_map_column()
->get_data()
.data();
+ if (null_map && (contains_file_type(from_type_not_nullable) ||
Review Comment:
[P2] Honor ancestor NULLs in JSON-to-container FILE casts
With `enable_strict_cast=true`, this reduced plan still evaluates a hidden
FILE payload:
```text
Project(CAST(IF(id=1, NULL, named_struct('a', j))
AS STRUCT<a:ARRAY<FILE>>))
Scan(id, j JSON)
```
Let row 1's `j` contain a six-field FILE object inside an array with
`size=-1`, and row 2 the same shape with a valid size.
`execute_for_null_then_else` retains row 1's JSON beneath the NULL STRUCT. This
merge forwards the ancestor mask through `create_struct_wrapper`, but
`create_cast_from_jsonb_wrapper` ignores its mask argument and deserializes
every JSON row. FILE validation then fails the whole query instead of returning
NULL for row 1. Skip masked rows in that JSONB-to-container conversion. This is
separate from thread 4262081668: no VARIANT bridge participates, and fixing
that bridge's second conversion does not fix this path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java:
##########
@@ -115,6 +123,35 @@ public List<Rule> buildRules() {
);
}
+ /** Validate operator keys after expression binding. */
+ public static void checkFilePlan(Plan plan) {
+ if (plan instanceof LogicalSort) {
+ ((LogicalSort<?>) plan).getOrderKeys()
+ .forEach(key ->
ExpressionUtils.checkFileKey(key.getExpr(), "ORDER BY"));
+ }
+ if (plan instanceof LogicalAggregate) {
Review Comment:
[P2] Validate aggregate DISTINCT arguments as FILE keys
Checking only explicit GROUP BY admits this reduced plan:
```text
Aggregate(groupBy=[], output=[ARRAY_AGG(DISTINCT f)])
Project(<nonconstant valid six-field FILE cast> AS f)
Numbers
```
`ARRAY_AGG` and `COLLECT_LIST` accept ANY with DISTINCT and lack COUNT's
separate complex-type restriction. `SplitAggMultiPhaseWithoutGbyKey` later
calls `AggregateUtils.getAllKeySet` and `splitDeduplicateOnePhase`, introducing
`PhysicalHashAggregate(groupBy=[f])`; the upper aggregate removes DISTINCT.
These queries therefore use FILE's serialized physical bytes for SQL
deduplication despite the new type explicitly excluding SQL equality/hash/dedup
semantics. Validate `getDistinctArguments()` recursively here before physical
splitting, while preserving non-distinct FILE collection. Include nested FILE
arguments in the negative coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java:
##########
@@ -133,6 +134,27 @@ public class ExpressionUtils {
// folding the conjunct). We restrict MAX_MARK_SLOT_COUNT to 4 to bound
this cost.
private static final int MAX_MARK_SLOT_COUNT = 4;
+ /** Reject FILE values in SQL keys that require comparison, ordering, or
hashing. */
+ public static void checkFileKey(Expression value, String operation) {
+ if (value.getDataType().typeContainsFile()) {
+ throw new AnalysisException(operation + " does not support FILE or
a type containing FILE: "
+ + value.getDataType().toSql());
+ }
+ }
+
+ /** Called by the existing recursive expression type check, without a
second plan traversal. */
+ public static void checkFileExpression(Expression expression) {
Review Comment:
[P2] Apply the FILE comparison restriction to NULLIF
These expression checks leave this reduced plan admitted:
```text
Project(NULLIF(f, g))
Project(<valid FILE cast with uri=concat('urn:left:',number)> AS f,
<valid FILE cast with uri=concat('urn:right:',number)> AS g)
Numbers
```
Use canonical six-field STRUCT casts with NULL optional children for `f` and
`g`. `NullIf.customSignature` accepts matching FILE types, and
`processBoundFunction` checks only VARBINARY for this function.
`FunctionNullIf` then executes `eq(f,g)`, whose generic path calls
`ColumnFile::compare_at` and throws `FILE comparison is not supported`. The
added `NULLIF(f,f)` regression only covers identical operands, which can be
folded to NULL and also have an identical-column shortcut in BE. Recursively
reject FILE in both NULLIF operands before coercion/folding and cover distinct
nonconstant inputs.
--
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]
