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]

Reply via email to