BePPPower commented on code in PR #24334:
URL: https://github.com/apache/doris/pull/24334#discussion_r1325371152


##########
be/src/vec/sink/writer/vfile_result_writer.cpp:
##########
@@ -172,9 +172,11 @@ Status VFileResultWriter::_create_file_writer(const 
std::string& file_name) {
 
 // file name format as: my_prefix_{fragment_instance_id}_0.csv
 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++) << "."

Review Comment:
   ```suggestion
       ss << _file_opts->file_path << print_id(_fragment_instance_id) << "_" << 
(_file_idx++) << "." << _file_opts->file_suffix;
   ```
   May be we can do this transformation in FE.
   So, here can just write like this.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/OutFileClause.java:
##########
@@ -156,6 +158,7 @@ public class OutFileClause {
     private TFileFormatType fileFormatType;
     private long maxFileSizeBytes = DEFAULT_MAX_FILE_SIZE_BYTES;
     private boolean deleteExistingFiles = false;
+    private String fileSuffix = null;

Review Comment:
   ```suggestion
       private String fileSuffix = "";
   ```
   I think there should not be null. Otherwise, 
`sinkOptions.setFileSuffix(fileSuffix);` it will give a `null` value to 
sinkOptions.



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to