github-actions[bot] commented on code in PR #68853:
URL: https://github.com/apache/doris/pull/68853#discussion_r4254140726
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java:
##########
@@ -156,6 +161,25 @@ public ColumnStatistic visit(Expression expr, Statistics
context) {
return expr.child(0).accept(this, context);
}
+ @Override
+ public ColumnStatistic visitIsNull(IsNull isNull, Statistics context) {
+ if (!isNull.child().getDataType().isFileType()) {
+ return visit(isNull, context);
+ }
+ ColumnStatistic child = estimate(isNull.child(), context);
+ double rowCount = context.getRowCount();
+ boolean canBeTrue = rowCount > 0 && (child.isUnKnown || child.numNulls
> 0);
Review Comment:
[P2] Keep FILE null-predicate ranges conservative for conditional values
`visitIf`/`visitCaseWhen` return known statistics with `numNulls=0` even for
nullable or unknown FILE branches, so this code turns a generic estimate into
an exact `[0,0]` Boolean range. A reduced reachable plan is:
```text
Filter(p = TRUE)
Project(p := ISNULL(IF(flag, TO_FILE('r', uri), CAST(NULL AS FILE))))
Scan(flag, uri)
```
With a valid S3 resource `r` and all `uri` values NULL, every row has
`p=TRUE`. The volatile producer keeps the filter above the projection, but
`estimateColumnEqualToConstant` estimates zero rows because TRUE lies outside
`[0,0]`. This affects cardinality/costing for positive IS NULL and is distinct
from the existing NOT/ISNULL thread. Preserve a conservative range when the
child's null count is estimated, or derive conditional null counts reliably;
cover nullable IF/CASE projections followed by a filter.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -846,6 +871,74 @@ && hasTimeStampNsCompatibleDateTimeType(argType)) {
});
}
+ /** Reject FILE operations without backend support, preserving value
propagation and construction. */
+ public static void checkFileFunction(BoundFunction function) {
+ String name = function.getName().toLowerCase(Locale.ROOT);
+ if (function instanceof Combinator) {
+ checkFileFunction(((Combinator) function).getNestedFunction());
+ }
+ if (function instanceof AggregateFunction && ((AggregateFunction)
function).isDistinct()) {
+ function.getArguments().forEach(arg ->
Expression.checkFileKey(arg, "DISTINCT " + name));
+ }
+ if (function instanceof Udf || function instanceof AliasUdf) {
+ if (function instanceof PythonUdf || function instanceof
PythonUdaf || function instanceof PythonUdtf) {
+ // Replayed functions bypass CREATE validation. Check declared
types too, since NULL
+ // arguments and scalar arguments cannot expose unsupported
return/intermediate types.
+ FunctionSignature signature = function.getSignatures().get(0);
+ org.apache.doris.catalog.Type intermediateType = null;
+ if (function instanceof PythonUdaf) {
+ intermediateType =
((org.apache.doris.catalog.AggregateFunction)
+ ((PythonUdaf)
function).getCatalogFunction()).getIntermediateType();
+ }
+ if (signature.returnType.typeContainsFile()
+ ||
signature.argumentsTypes.stream().anyMatch(DataType::typeContainsFile)
+ || (intermediateType != null &&
intermediateType.typeContainsFile())) {
+ throw new AnalysisException("PYTHON_UDF does not support
FILE or a type containing FILE");
+ }
+ } else {
+ FunctionSignature signature = function.getSignatures().get(0);
+ if (signature.returnType.typeContainsFile()
+ ||
signature.argumentsTypes.stream().anyMatch(DataType::typeContainsFile)) {
+ throw new AnalysisException(name + " does not support FILE
or a type containing FILE");
+ }
+ }
+ }
+ if (!(function instanceof Count || function instanceof ElementAt ||
function instanceof ToFile
+ || function instanceof ExplodeFile
+ || function instanceof ExplodeFileOuter ||
supportsFileValuePropagation(function))) {
+ function.getArguments().forEach(arg -> {
+ if (arg instanceof Lambda) {
+ // Higher-order binding packs real input arrays into the
lambda closure.
+ Lambda lambda = (Lambda) arg;
+ Expression.checkFileKey(lambda.getLambdaFunction(), name);
+ lambda.getLambdaArguments().forEach(argument ->
+
Expression.checkFileKey(argument.getArrayExpression(), name));
+ } else {
+ Expression.checkFileKey(arg, name);
+ }
+ });
+ }
+ // MAP values can carry FILE, but MAP keys still require comparison
and hashing.
+ if (function.getArguments().stream().anyMatch(arg ->
arg.getDataType().typeContainsFile())
+ && function.getDataType().isMapType()) {
+ MapType map = (MapType) function.getDataType();
+ if (map.getKeyType().typeContainsFile()) {
+ throw new AnalysisException("FILE cannot be used in MAP keys");
+ }
+ }
+ }
+
+ private static boolean supportsFileValuePropagation(BoundFunction
function) {
+ // These backend implementations select/copy columns or inspect
container offsets;
+ // none compares, sorts or hashes FILE values. Keep type-specific
functions gated.
+ return function instanceof Coalesce || function instanceof If ||
function instanceof Nvl
+ || function instanceof Array || function instanceof
CreateStruct
+ || function instanceof CreateNamedStruct || function
instanceof CreateMap
+ || function instanceof ArrayConcat || function instanceof
ArraySlice || function instanceof Reverse
Review Comment:
[P2] Avoid repeated constant expansion before enabling FILE array concat
This newly admits `ARRAY_CONCAT` for FILE, but its existing BE kernel calls
`convert_to_full_column_if_const()` inside the row/argument loops
(`function_array_concat.cpp:91-99`). A reduced supported shape is:
```text
Project(array_concat(array(f), array(if(number % 2 = 0, f, NULL))))
Numbers(1024)
```
Here `f` is a constant valid FILE containing 64 KiB of inline bytes. The
varying second argument bypasses the all-constant fast path, so the first
`ColumnConst` is expanded to the entire batch anew for every row.
`ColumnArray::insert_many_from` reaches `ColumnFile` child insertion and
`ColumnVarbinary`'s deep arena copy: approximately 64 GiB of inline copying to
produce 64 MiB of that argument's output for a 1024-row batch (static
copy-volume arithmetic, not a benchmark). This makes the newly supported FILE
path quadratic in batch size. Unpack constant arrays once and index their
single row, or materialize each argument once outside the row loop, before
enabling this propagation path; cover mixed constant/dynamic arrays with
populated inline bytes.
--
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]
