github-actions[bot] commented on code in PR #66221:
URL: https://github.com/apache/doris/pull/66221#discussion_r3710897667
##########
thirdparty/build-thirdparty.sh:
##########
@@ -1088,9 +1090,7 @@ build_arrow() {
ldflags="-L${TP_LIB_DIR}"
fi
- CPPFLAGS="-I${TP_INCLUDE_DIR}" \
- CXXFLAGS="-I${TP_INCLUDE_DIR}" \
- LDFLAGS="${ldflags}" \
+ LDFLAGS="${ldflags}" \
Review Comment:
[P1] Keep the staged LZO headers on Arrow's compile path
The Arrow 24 LZO patch adds `#include <lzo/lzo1x.h>` to `column_reader.cc`,
while `build_lzo2()` installs that header under
`${TP_INSTALL_DIR}/include/lzo`. This hunk removes both `CPPFLAGS` and
`CXXFLAGS=-I${TP_INCLUDE_DIR}`, and no Arrow CMake target or supported
workflow/image supplies that directory during `build_arrow()` (the macOS
`CPPFLAGS` are exported only afterward). A clean or focused Arrow build
therefore fails compiling this translation unit with `lzo/lzo1x.h` not found.
Please retain `${TP_INCLUDE_DIR}` in Arrow's C++ flags or add it explicitly to
the patched Parquet target, and cover the real clean Arrow compile path in CI.
##########
thirdparty/patches/apache-arrow-24.0.0-paimon.patch:
##########
@@ -40,26 +36,23 @@ index 285e2a5973..aa6f92f077 100644
this]() mutable -> ::arrow::Result<RecordBatchIterator> {
::arrow::ChunkedArrayVector columns(readers.size());
-- // don't reserve more rows than necessary
+ // don't reserve more rows than necessary
- int64_t batch_size = std::min(properties().batch_size(), num_rows);
- num_rows -= batch_size;
+ int64_t batch_size = 0;
+ if (!num_rows.empty()) {
-+ // don't reserve more rows than necessary
+ batch_size = std::min(properties().batch_size(),
num_rows[row_group_idx]);
+ num_rows[row_group_idx] -= batch_size;
-+ if (num_rows[row_group_idx] == 0 && (num_rows.size() - 1) !=
row_group_idx) {
-+ row_group_idx++;
++ if (num_rows[row_group_idx] == 0 && row_group_idx + 1 <
num_rows.size()) {
Review Comment:
[P1] Do not turn a zero-row group into end of stream
A selected zero-row group enters this branch with `batch_size == 0`,
advances `row_group_idx`, and still calls every leaf reader with
`NextBatch(0)`. The unchanged Arrow lambda then sees zero-length columns and
returns iterator end, so a later nonempty row group is never read. This is
reachable with valid metadata: Arrow 24 can write a zero-row group for an empty
table, and Paimon's pinned `FileReaderWrapper::Create` selects every metadata
row group without filtering empties. Please skip/filter empty groups while
keeping Paimon's range index aligned, and add a regression with an empty group
followed by a populated one.
--
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]
