Mryange commented on PR #63386:
URL: https://github.com/apache/doris/pull/63386#issuecomment-4495574981
> I reviewed the current PR diff and the existing review threads. I am not
adding a duplicate inline comment, but the existing thread on
`be/src/core/column/column_array_view.h` remains blocking:
`ColumnArrayView::create()` assumes the array nested data column is always
`ColumnNullable`, while existing `ColumnArray` callers and tests support
non-nullable nested data. The new `array_join` and array distance paths now
depend on this helper, so non-nullable `Array<String>` / `Array<Float>` inputs
can fail instead of executing.
>
> Critical checkpoint conclusions:
>
> * Goal and tests: the refactor goal is clear, but the current
implementation does not preserve all supported array column shapes; tests only
cover nullable nested data and miss the non-nullable nested case already raised.
> * Scope: the main helper is focused, but it changes shared behavior for
multiple functions, so preserving the old `extract_column_array_info()`
semantics is required.
> * Concurrency/lifecycle: no new concurrency, locking, or lifecycle risks
found in the reviewed PR files.
> * Configuration/compatibility/persistence: no new config, storage format,
EditLog, or FE/BE protocol compatibility concerns found in the reviewed PR
files.
> * Parallel paths: `array_join` and distance functions are both affected by
the same helper assumption; no additional distinct location beyond the existing
thread was found.
> * Error handling/memory: no additional ignored `Status`,
exception-boundary, or tracked-memory issues found.
> * Performance: no additional blocking performance issue found; the extra
abstraction looks intended and benchmarked, pending correctness fix.
> * Test coverage: add coverage for non-nullable nested `ColumnArray` data,
because that is the uncovered regression scenario.
>
> User focus: no additional user-provided review focus was specified.
对于这些函数,他的array<T>中的T就是一个nullable的。另外你可以看一下data type array的内部,他已经是
DataTypeNullablePtr nested;的定义了。
所以不要在纠结这个问题了,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: [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]