github-actions[bot] commented on code in PR #67320:
URL: https://github.com/apache/doris/pull/67320#discussion_r3891456247
##########
thirdparty/patches/apache-arrow-17.0.0-paimon.patch:
##########
@@ -0,0 +1,224 @@
+diff --git a/cpp/src/parquet/arrow/schema.cc b/cpp/src/parquet/arrow/schema.cc
+index ec3890a41f..943f69bb6c 100644
+--- a/cpp/src/parquet/arrow/schema.cc
++++ b/cpp/src/parquet/arrow/schema.cc
+@@ -178,7 +178,7 @@ static Status GetTimestampMetadata(const
::arrow::TimestampType& type,
+
+ // The user is explicitly asking for Impala int96 encoding, there is no
+ // logical type.
+- if (arrow_properties.support_deprecated_int96_timestamps()) {
++ if (arrow_properties.support_deprecated_int96_timestamps() && target_unit
== ::arrow::TimeUnit::NANO) {
+ *physical_type = ParquetType::INT96;
+ return Status::OK();
+ }
+
+diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc
+index 285e2a5973..aa6f92f077 100644
+--- a/cpp/src/parquet/arrow/reader.cc
++++ b/cpp/src/parquet/arrow/reader.cc
+@@ -1013,25 +1013,32 @@ Status FileReaderImpl::GetRecordBatchReader(const
std::vector<int>& row_groups,
+ return Status::OK();
+ }
+
+- int64_t num_rows = 0;
++ std::vector<int64_t> num_rows;
+ for (int row_group : row_groups) {
+- num_rows += parquet_reader()->metadata()->RowGroup(row_group)->num_rows();
++
num_rows.push_back(parquet_reader()->metadata()->RowGroup(row_group)->num_rows());
+ }
+
+ using ::arrow::RecordBatchIterator;
++ int row_group_idx = 0;
+
+ // NB: This lambda will be invoked outside the scope of this call to
+ // `GetRecordBatchReader()`, so it must capture `readers` and
`batch_schema` by value.
+ // `this` is a non-owning pointer so we are relying on the parent
FileReader outliving
+ // this RecordBatchReader.
+ ::arrow::Iterator<RecordBatchIterator> batches =
::arrow::MakeFunctionIterator(
+- [readers, batch_schema, num_rows,
++ [readers, batch_schema, num_rows, row_group_idx,
+ this]() mutable -> ::arrow::Result<RecordBatchIterator> {
+ ::arrow::ChunkedArrayVector columns(readers.size());
+
+- // 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;
Review Comment:
[P1] Do not terminate the iterator on an empty row group
With a zero-row first selected group, this computes `batch_size = 0`,
advances the index, then calls `NextBatch(0)`. The existing empty-column check
immediately returns `IterationTraits::End()`, so later non-empty groups are
silently skipped. Paimon passes row-group IDs directly, making `[empty,
non-empty]` metadata reachable. Skip exhausted groups before `NextBatch`, and
return end only after all selected groups are exhausted; add a regression for
this ordering.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -1062,11 +1068,19 @@ build_grpc() {
# sed -i 's/find_dependency/find_package/g'
"${TP_INSTALL_DIR}"/lib64/cmake/grpc/gRPCConfig.cmake
}
-# arrow
-build_arrow() {
- check_if_source_exist "${ARROW_SOURCE}"
- invalidate_arrow_prebuilt_marker "${TP_INSTALL_DIR}"
- cd "${TP_SOURCE_DIR}/${ARROW_SOURCE}/cpp"
+# Arrow 17 is installed in the legacy unversioned prefix for pre-upgrade
+# branch-4.1 revisions, while Arrow 24 is installed in a versioned prefix
+# selected by master.
+build_arrow_stack() {
+ local arrow_source="$1"
+ local xsimd_archive="$2"
+ local install_dir="$3"
+ local has_separate_compute_archive="$4"
+
+ check_if_source_exist "${arrow_source}"
+ mkdir -p "${install_dir}/lib64"
+ ln -sfn lib64 "${install_dir}/lib"
Review Comment:
[P2] Handle an existing real `lib` directory before linking `lib64`
`ln -sfn lib64 "${install_dir}/lib"` does not replace a real directory; it
creates `lib/lib64` instead. CMake installs the archives under `lib64`, but
`strip_lib_at` then looks under `lib`, so a reused prefix with a real `lib`
directory fails during stripping or leaves the prefix split. Detect/migrate or
reject a non-symlink `lib` path before creating the link, and assert the
resolved path before stripping. The Paimon builder repeats the same operation
at line 2085.
##########
thirdparty/arrow-paimon-vars.sh:
##########
@@ -191,6 +236,57 @@ arrow_paimon_fingerprint_matches() {
return 1
}
+arrow_17_build_fingerprint() {
+ local vars_dir
+ vars_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ (
+ set -o pipefail
+ cd "${vars_dir}" || return 1
+ {
+ printf 'schema=%s\n' "${ARROW_BUILD_SCHEMA_VERSION}"
+ printf 'ARROW_17_VERSION=%s\n' "${ARROW_17_VERSION}"
+ printf 'ARROW_17_NAME=%s\n' "${ARROW_17_NAME}"
+ printf 'ARROW_17_SOURCE=%s\n' "${ARROW_17_SOURCE}"
+ printf 'ARROW_17_MD5SUM=%s\n' "${ARROW_17_MD5SUM}"
+ printf 'BROTLI_NAME=%s\n' "${BROTLI_NAME}"
+ printf 'BROTLI_SOURCE=%s\n' "${BROTLI_SOURCE}"
+ printf 'BROTLI_MD5SUM=%s\n' "${BROTLI_MD5SUM}"
+ printf 'XSIMD_17_NAME=%s\n' "${XSIMD_17_NAME}"
+ printf 'XSIMD_17_SOURCE=%s\n' "${XSIMD_17_SOURCE}"
+ printf 'XSIMD_17_MD5SUM=%s\n' "${XSIMD_17_MD5SUM}"
+ arrow_paimon_fingerprint_files \
Review Comment:
[P1] Validate both Arrow/Paimon stacks before accepting a shared prebuilt
This retargets the artifact validator to `installed/arrow-24.0.0`, but
`install_arrow_paimon_prebuilt_archive` and
`ensure_arrow_paimon_prebuilt_from_url` still use that single-stack validator.
Because the downloaded archive replaces the entire `installed` tree, an archive
with valid Arrow 24/Paimon 24 artifacts but no root Arrow 17/Paimon 17 stack is
accepted and can strand branch-4.1 consumers; the existing-tree check also
short-circuits on the same partial state. Use
`shared_arrow_paimon_prebuilt_valid` for the archive and existing-tree checks
(or preserve/merge the legacy stack), and add a missing-Arrow-17 archive
regression.
--
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]
