hubgeter commented on code in PR #67320:
URL: https://github.com/apache/doris/pull/67320#discussion_r3891762630
##########
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:
Fixed in 4182e7c23c1. Archive candidates, existing installs, and final
installs now all use shared_arrow_paimon_prebuilt_valid, so both the legacy
Arrow/Paimon 17 stack and versioned Arrow/Paimon 24 stack are required. The
lifecycle test now covers both a current-only existing tree and a downloaded
archive missing Arrow 17; it passes locally.
##########
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:
Fixed in 4182e7c23c1. The Arrow and Paimon stack builders no longer create
install/lib with ln -sfn. strip_lib_at now receives the exact library
directory, and Arrow explicitly strips from install/lib64. A real four-stack
build completed for arrow_17, paimon_cpp_17, arrow, and paimon_cpp, with no
nested lib/lib64 path.
##########
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:
Fixed in 4182e7c23c1 for both Arrow 17 and Arrow 24. The iterator skips
zero-length row groups before calling NextBatch and returns End only after all
selected groups are exhausted. Added
GetRecordBatchReaderSkipsEmptyLeadingRowGroup to both patch variants; both
targeted upstream tests pass locally.
--
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]
