This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 4ffe6632b8c [feature](outfile) add opt-in file_name_padding (#68746)
4ffe6632b8c is described below
commit 4ffe6632b8cf75a0d647719c5f7a5f77308f0e5f
Author: Alvaro Tuso <[email protected]>
AuthorDate: Thu Oct 15 05:04:08 2026 -0300
[feature](outfile) add opt-in file_name_padding (#68746)
### What problem does this PR solve?
Files generated by `SELECT INTO OUTFILE` and `TVF` have a suffix that is
not lexicographically sorted, which makes listing files from object
storage awkward for queries with `ORDER BY`, as they will come back out
of order.
This PR adds a new backwards compatible parameter called
`file_name_padding` that allows padding file names with `0`s in a
similar fashion to what spark and trino do. The value is configurable
from `0` (no padding, same behavior as today) to `10` (max int width)
Files with the default behavior will sort like this:
```
_9
_10
_100
_11
```
While files with appropriate padding will sort like this, allowing the
file names to be ordered in the same manner as the query results:
```
_00009
_00010
_00011
...
_00099
_00100
```
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
Co-authored-by: Alvaro Tuso <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
be/src/exec/operator/result_sink_operator.h | 6 ++
be/src/exec/sink/writer/result_file_name.h | 39 ++++++++++++
be/src/exec/sink/writer/vfile_result_writer.cpp | 7 +--
be/src/exec/sink/writer/vtvf_table_writer.cpp | 6 +-
be/src/exec/sink/writer/vtvf_table_writer.h | 1 +
be/test/exec/sink/writer/result_file_name_test.cpp | 73 ++++++++++++++++++++++
.../org/apache/doris/analysis/OutFileClause.java | 20 ++++++
.../org/apache/doris/planner/TVFTableSink.java | 21 +++++++
gensrc/thrift/DataSinks.thrift | 9 +++
9 files changed, 176 insertions(+), 6 deletions(-)
diff --git a/be/src/exec/operator/result_sink_operator.h
b/be/src/exec/operator/result_sink_operator.h
index 51be25c02a6..bada2ed61c0 100644
--- a/be/src/exec/operator/result_sink_operator.h
+++ b/be/src/exec/operator/result_sink_operator.h
@@ -65,6 +65,9 @@ struct ResultFileOptions {
//Bring BOM when exporting to CSV format
bool with_bom = false;
int64_t orc_writer_version = 0;
+ // Zero-pad the per-file index in the output file name to this many digits.
+ // 0 means no padding, preserving the historical name format.
+ int file_name_padding = 0;
ResultFileOptions(const TResultFileSinkOptions& t_opt) {
file_path = t_opt.file_path;
@@ -78,6 +81,9 @@ struct ResultFileOptions {
t_opt.__isset.delete_existing_files ?
t_opt.delete_existing_files : false;
file_suffix = t_opt.file_suffix;
with_bom = t_opt.with_bom;
+ if (t_opt.__isset.file_name_padding) {
+ file_name_padding = t_opt.file_name_padding;
+ }
is_local_file = true;
if (t_opt.__isset.broker_addresses) {
diff --git a/be/src/exec/sink/writer/result_file_name.h
b/be/src/exec/sink/writer/result_file_name.h
new file mode 100644
index 00000000000..111061bb48e
--- /dev/null
+++ b/be/src/exec/sink/writer/result_file_name.h
@@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <fmt/format.h>
+
+#include <string>
+
+namespace doris {
+
+// Builds an output file name of the form "{prefix}{id}_{idx}.{suffix}",
shared by
+// the SELECT INTO OUTFILE writer (VFileResultWriter) and the INSERT INTO
FUNCTION
+// TVF writer (VTVFTableWriter).
+//
+// When pad > 0 the index is zero-padded to `pad` digits so that the files
produced
+// by a single writer sort lexicographically (e.g. _00011 sorts after _00002
rather
+// than before _2). pad <= 0 leaves the index unpadded, which is byte-for-byte
the
+// historical name format.
+inline std::string build_result_file_name(const std::string& prefix, const
std::string& id, int idx,
+ int pad, const std::string& suffix) {
+ return fmt::format("{}{}_{:0{}}.{}", prefix, id, idx, pad < 0 ? 0 : pad,
suffix);
+}
+
+} // namespace doris
diff --git a/be/src/exec/sink/writer/vfile_result_writer.cpp
b/be/src/exec/sink/writer/vfile_result_writer.cpp
index ce9e73d2eba..4528eb92360 100644
--- a/be/src/exec/sink/writer/vfile_result_writer.cpp
+++ b/be/src/exec/sink/writer/vfile_result_writer.cpp
@@ -38,6 +38,7 @@
#include "core/value/decimalv2_value.h"
#include "core/value/large_int_value.h"
#include "exec/operator/result_sink_operator.h"
+#include "exec/sink/writer/result_file_name.h"
#include "exec/sink/writer/vmysql_result_writer.h"
#include "exprs/function/cast/cast_to_string.h"
#include "exprs/vexpr.h"
@@ -162,10 +163,8 @@ Status VFileResultWriter::_create_file_writer(const
std::string& file_name) {
Status VFileResultWriter::_get_next_file_name(std::string* file_name) {
std::string suffix =
_file_opts->file_suffix.empty() ? _file_format_to_name() :
_file_opts->file_suffix;
- std::stringstream ss;
- ss << _file_opts->file_path << print_id(_fragment_instance_id) << "_" <<
(_file_idx++) << "."
- << suffix;
- *file_name = ss.str();
+ *file_name = build_result_file_name(_file_opts->file_path,
print_id(_fragment_instance_id),
+ _file_idx++,
_file_opts->file_name_padding, suffix);
if (_storage_type == TStorageBackendType::LOCAL) {
// For local file writer, the file_path is a local dir.
// Here we do a simple security verification by checking whether the
file exists.
diff --git a/be/src/exec/sink/writer/vtvf_table_writer.cpp
b/be/src/exec/sink/writer/vtvf_table_writer.cpp
index c4111c7c2d3..137279c8c32 100644
--- a/be/src/exec/sink/writer/vtvf_table_writer.cpp
+++ b/be/src/exec/sink/writer/vtvf_table_writer.cpp
@@ -21,6 +21,7 @@
#include "common/status.h"
#include "core/block/block.h"
+#include "exec/sink/writer/result_file_name.h"
#include "exprs/vexpr.h"
#include "exprs/vexpr_context.h"
#include "io/file_factory.h"
@@ -48,6 +49,7 @@ Status VTVFTableWriter::open(RuntimeState* state,
RuntimeProfile* profile) {
_file_path = _tvf_sink.file_path;
_max_file_size_bytes =
_tvf_sink.__isset.max_file_size_bytes ?
_tvf_sink.max_file_size_bytes : 0;
+ _file_name_padding = _tvf_sink.__isset.file_name_padding ?
_tvf_sink.file_name_padding : 0;
VLOG_DEBUG << "TVF table writer open, query_id=" <<
print_id(_state->query_id())
<< ", tvf_name=" << _tvf_sink.tvf_name << ", file_path=" <<
_tvf_sink.file_path
@@ -164,8 +166,8 @@ Status VTVFTableWriter::_get_next_file_name(std::string*
file_name) {
// file_path is a prefix, generate: {prefix}{query_id}_{idx}.{ext}
std::string query_id_str = print_id(_state->query_id());
- *file_name = fmt::format("{}{}_{}.{}", _file_path, query_id_str,
_file_idx, ext);
- _file_idx++;
+ *file_name =
+ build_result_file_name(_file_path, query_id_str, _file_idx++,
_file_name_padding, ext);
return Status::OK();
}
diff --git a/be/src/exec/sink/writer/vtvf_table_writer.h
b/be/src/exec/sink/writer/vtvf_table_writer.h
index 54fabeecd5f..a215a21c4b5 100644
--- a/be/src/exec/sink/writer/vtvf_table_writer.h
+++ b/be/src/exec/sink/writer/vtvf_table_writer.h
@@ -72,6 +72,7 @@ private:
int64_t _current_written_bytes = 0;
int64_t _max_file_size_bytes = 0;
int _file_idx = 0;
+ int _file_name_padding = 0;
std::string _file_path;
// profile counters
diff --git a/be/test/exec/sink/writer/result_file_name_test.cpp
b/be/test/exec/sink/writer/result_file_name_test.cpp
new file mode 100644
index 00000000000..9bb533edf64
--- /dev/null
+++ b/be/test/exec/sink/writer/result_file_name_test.cpp
@@ -0,0 +1,73 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "exec/sink/writer/result_file_name.h"
+
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <string>
+#include <vector>
+
+namespace doris {
+
+TEST(ResultFileNameTest, no_padding_preserves_legacy_format) {
+ EXPECT_EQ("s3://bucket/exp_abc_0.csv",
+ build_result_file_name("s3://bucket/exp_", "abc", 0, 0, "csv"));
+ EXPECT_EQ("s3://bucket/exp_abc_11.csv",
+ build_result_file_name("s3://bucket/exp_", "abc", 11, 0, "csv"));
+}
+
+TEST(ResultFileNameTest, negative_padding_is_treated_as_none) {
+ EXPECT_EQ("s3://bucket/exp_abc_7.csv",
+ build_result_file_name("s3://bucket/exp_", "abc", 7, -1, "csv"));
+}
+
+TEST(ResultFileNameTest, padding_zero_fills_to_width) {
+ EXPECT_EQ("s3://bucket/exp_abc_00000.csv",
+ build_result_file_name("s3://bucket/exp_", "abc", 0, 5, "csv"));
+ EXPECT_EQ("s3://bucket/exp_abc_00011.csv",
+ build_result_file_name("s3://bucket/exp_", "abc", 11, 5, "csv"));
+}
+
+TEST(ResultFileNameTest, padding_does_not_truncate_when_index_exceeds_width) {
+ // Width is a minimum, never a cap: an index wider than `pad` is emitted
in full.
+ EXPECT_EQ("s3://bucket/exp_abc_123456.csv",
+ build_result_file_name("s3://bucket/exp_", "abc", 123456, 5,
"csv"));
+}
+
+TEST(ResultFileNameTest, padded_names_sort_lexicographically) {
+ // The bug this feature fixes: without padding, _11 sorts before _2.
+ std::vector<std::string> unpadded;
+ std::vector<std::string> padded;
+ for (int i : {0, 1, 2, 10, 11, 100}) {
+ unpadded.push_back(build_result_file_name("exp_", "id", i, 0, "csv"));
+ padded.push_back(build_result_file_name("exp_", "id", i, 5, "csv"));
+ }
+
+ std::vector<std::string> unpadded_sorted = unpadded;
+ std::sort(unpadded_sorted.begin(), unpadded_sorted.end());
+ // Lexicographic sort scrambles the numeric order for unpadded names.
+ EXPECT_NE(unpadded, unpadded_sorted);
+
+ std::vector<std::string> padded_sorted = padded;
+ std::sort(padded_sorted.begin(), padded_sorted.end());
+ // Padded names keep numeric order under a lexicographic sort.
+ EXPECT_EQ(padded, padded_sorted);
+}
+
+} // namespace doris
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/OutFileClause.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/OutFileClause.java
index 64311e202c7..68ef656f1d1 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/OutFileClause.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/OutFileClause.java
@@ -118,11 +118,15 @@ public class OutFileClause {
public static final String PROP_DELETE_EXISTING_FILES =
"delete_existing_files";
public static final String PROP_FILE_SUFFIX = "file_suffix";
public static final String PROP_WITH_BOM = "with_bom";
+ public static final String PROP_FILE_NAME_PADDING = "file_name_padding";
private static final String SCHEMA = "schema";
private static final long DEFAULT_MAX_FILE_SIZE_BYTES = 1 * 1024 * 1024 *
1024; // 1GB
private static final long MIN_FILE_SIZE_BYTES = 5 * 1024 * 1024L; // 5MB
+ // Upper bound on the zero-padding width for the per-file index in output
file
+ // names. _file_idx on the BE is a 32-bit int, so 10 digits covers its
full range.
+ private static final int MAX_FILE_NAME_PADDING = 10;
private String filePath;
private Map<String, String> properties;
@@ -131,6 +135,7 @@ public class OutFileClause {
private boolean deleteExistingFiles = false;
private String fileSuffix = "";
private boolean withBom = false;
+ private int fileNamePadding = 0;
private BrokerDesc brokerDesc = null;
// True if result is written to local disk.
// If set to true, the brokerDesc must be null.
@@ -553,6 +558,20 @@ public class OutFileClause {
copiedProps.remove(PROP_WITH_BOM);
}
+ if (copiedProps.containsKey(PROP_FILE_NAME_PADDING)) {
+ try {
+ fileNamePadding =
Integer.parseInt(copiedProps.get(PROP_FILE_NAME_PADDING));
+ } catch (NumberFormatException e) {
+ throw new AnalysisException(PROP_FILE_NAME_PADDING + " must be
an integer. Given: "
+ + copiedProps.get(PROP_FILE_NAME_PADDING));
+ }
+ if (fileNamePadding < 0 || fileNamePadding >
MAX_FILE_NAME_PADDING) {
+ throw new AnalysisException(PROP_FILE_NAME_PADDING + " must be
between 0 and "
+ + MAX_FILE_NAME_PADDING + ". Given: " +
fileNamePadding);
+ }
+ copiedProps.remove(PROP_FILE_NAME_PADDING);
+ }
+
if (copiedProps.containsKey(PROP_SUCCESS_FILE_NAME)) {
successFileName = copiedProps.get(PROP_SUCCESS_FILE_NAME);
FeNameFormat.checkOutfileSuccessFileName("file name",
successFileName);
@@ -758,6 +777,7 @@ public class OutFileClause {
sinkOptions.setDeleteExistingFiles(deleteExistingFiles);
sinkOptions.setFileSuffix(fileSuffix);
sinkOptions.setWithBom(withBom);
+ sinkOptions.setFileNamePadding(fileNamePadding);
if (brokerDesc != null) {
sinkOptions.setBrokerProperties(brokerDesc.getBackendConfigProperties());
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/planner/TVFTableSink.java
b/fe/fe-core/src/main/java/org/apache/doris/planner/TVFTableSink.java
index 52189daca14..507e1c6595c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/TVFTableSink.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/TVFTableSink.java
@@ -48,6 +48,11 @@ import java.util.Map;
* infrastructure as the read-side TVF (SELECT * FROM s3/hdfs/local(...)).
*/
public class TVFTableSink extends DataSink {
+ // Upper bound on the zero-padding width for the per-file index in output
file names
+ // (the "file_name_padding" property). _file_idx on the BE is a 32-bit
int, so 10 digits
+ // covers its full range.
+ private static final int MAX_FILE_NAME_PADDING = 10;
+
private final PlanNodeId exchNodeId;
private final String tvfName;
private final Map<String, String> properties;
@@ -156,6 +161,22 @@ public class TVFTableSink extends DataSink {
tSink.setMaxFileSizeBytes(Long.parseLong(maxFileSizeStr));
}
+ // Zero-pad the per-file index in output file names so they sort
lexicographically.
+ String fileNamePaddingStr = properties.get("file_name_padding");
+ if (fileNamePaddingStr != null) {
+ int fileNamePadding;
+ try {
+ fileNamePadding = Integer.parseInt(fileNamePaddingStr);
+ } catch (NumberFormatException e) {
+ throw new AnalysisException("file_name_padding must be an
integer. Given: " + fileNamePaddingStr);
+ }
+ if (fileNamePadding < 0 || fileNamePadding >
MAX_FILE_NAME_PADDING) {
+ throw new AnalysisException("file_name_padding must be between
0 and "
+ + MAX_FILE_NAME_PADDING + ". Given: " +
fileNamePadding);
+ }
+ tSink.setFileNamePadding(fileNamePadding);
+ }
+
// Delete existing files is handled by FE (InsertIntoTVFCommand),
always tell BE not to delete
tSink.setDeleteExistingFiles(false);
diff --git a/gensrc/thrift/DataSinks.thrift b/gensrc/thrift/DataSinks.thrift
index 4ac527df0ef..6ad86202e07 100644
--- a/gensrc/thrift/DataSinks.thrift
+++ b/gensrc/thrift/DataSinks.thrift
@@ -160,6 +160,12 @@ struct TResultFileSinkOptions {
// currently only for csv
// TODO: merge with parquet_compression_type and orc_compression_type
22: optional PlanNodes.TFileCompressType compression_type
+
+ // Zero-pad the per-file index in the output file name to this many digits
+ // (e.g. my_prefix_{fragment_instance_id}_00000.csv). 0 or unset means no
+ // padding, preserving the historical name format. Used to make rollover
+ // files sort lexicographically (so _00011 sorts after _00002).
+ 23: optional i32 file_name_padding
}
struct TMemoryScratchSink {
@@ -618,6 +624,9 @@ struct TTVFTableSink {
// Java class name: a plugin's classes are private to its own classloader,
so BE addresses a
// writer by the plugin directory it is deployed in and the factory name
within it.
15: optional string writer_class
+ // Zero-pad the per-file index in the output file name to this many digits.
+ // 0 or unset means no padding. Mirrors
TResultFileSinkOptions.file_name_padding.
+ 16: optional i32 file_name_padding
}
struct TMCCommitData {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
