From 887b62fd1d91204ca7421decd9832c974480a85f Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Tue, 21 Jul 2026 11:58:24 +0800 Subject: [PATCH 1/2] [MINOR][CH] Correct misleading Spark 3.2 comment in ReadBufferBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fallback path at `if (!file_size.has_value()) { ... }` was commented as "only for spark3.2 file partition not contained file size", but the fallback is still triggered on Spark 3.3+ whenever the Substrait `LocalFilesNode` was built without a `fileSizes` list — e.g. by `IcebergLocalFilesBuilder`. Reword the comment to describe the actual condition. No code change. --- .../Storages/SubstraitSource/ReadBufferBuilder.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp b/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp index 6d999657098..3a3f8bd1daa 100644 --- a/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp +++ b/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp @@ -326,8 +326,9 @@ class HDFSFileReadBufferBuilder : public ReadBufferBuilder { if (!file_size.has_value()) { - // only for spark3.2 file partition not contained file size - // so first compute file size first + // Fallback for callers whose Substrait LocalFilesNode was built without a + // fileSize list (e.g. some Iceberg producers). Compute the size by opening + // the HDFS file first. auto tmp_read_buffer = std::make_unique( hdfs_uri, hdfs_file_path, From 8cccb46649860410f6ce59617d7e37398735830d Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Tue, 21 Jul 2026 12:41:16 +0800 Subject: [PATCH 2/2] [MINOR] Reword ReadBufferBuilder fallback comment for accuracy Address Copilot review feedback on #12583: distinguish the producer-side identifier (`fileSizes` list in `LocalFilesNode`) from the per-file protobuf property (`fileSize`) so the comment matches the code. --- .../Storages/SubstraitSource/ReadBufferBuilder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp b/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp index 3a3f8bd1daa..3ea544dc01d 100644 --- a/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp +++ b/cpp-ch/local-engine/Storages/SubstraitSource/ReadBufferBuilder.cpp @@ -327,8 +327,8 @@ class HDFSFileReadBufferBuilder : public ReadBufferBuilder if (!file_size.has_value()) { // Fallback for callers whose Substrait LocalFilesNode was built without a - // fileSize list (e.g. some Iceberg producers). Compute the size by opening - // the HDFS file first. + // `fileSizes` input (e.g. some Iceberg producers), so no per-file `fileSize` + // property is available. Compute the size by opening the HDFS file first. auto tmp_read_buffer = std::make_unique( hdfs_uri, hdfs_file_path,