fix(parquet): write single file if option is set

[hknlof]

• Regression: DataFrameWriteOptions::with_single_file_output produces a directory #13323

Which issue does this PR close?

• Closes Regression: DataFrameWriteOptions::with_single_file_output produces a directory #13323

Rationale for this change

DF.write_parquet writes multiple files / one directory even if options.single_file_output is set.

What changes are included in this PR?

Introduce an internal .single extension.

Are these changes tested?

Yes, tests are part of this PR.

Are there any user-facing changes?

Not in this implementation. There might be, if we decide to move to an FileSinkConfig based solution.

Quoting: #13323 (comment)

It seems hard to control the behavior of write_parquet by single_file_output(and I've noticed that It's never used), what really controls whether to generate a single file output is determining the suffix(in start_demuxer_task()), there are several methods I can think of to handle this issue:

1. We can add a suffix like .single to the paths that require generating a single file, and then recognize this suffix in start_demuxer_task().
2. Give up single_file_output in DataFrameWriteOptions, use FileSinkConfig instead to control single file behavior.

[alamb]

I restarted the checks

[alamb]

Sorry for the delay -- this PR seems to have quite. few conflicts.

[hknlof]

Sorry for the delay -- this PR seems to have quite. few conflicts.

No worries! My bad, I forgot to sync before rebasing the branch onto main again. Thx!

[Jefffrey]

This way of implicitly encoding single fileness via adding an extension to the path does seem quite hacky; not sure if this is the correct approach to follow 🤔

[Jefffrey]

This part confuses me; If I'm not wrong file_type.get_ext() != DEFAULT_PARQUET_EXTENSION will always be true, because:

datafusion/datafusion/datasource-parquet/src/file_format.rs

Lines 157 to 162 in 602475f

	impl GetExt for ParquetFormatFactory {
	fn get_ext(&self) -> String {
	// Removes the dot, i.e. ".parquet" -> "parquet"
	DEFAULT_PARQUET_EXTENSION[1..].to_string()
	}
	}

[Jefffrey]

I think we would need to also check there are indeed multiple parquet files

[Jefffrey]

Similarly here we would need to check that only one file is written, not just a file is written

[alamb]

@hknlof do you think you will be able to address @Jefffrey 's comments? I think this ticket is marked as ideally including in DataFusion 51 which we hope to cut next week

[hknlof]

@alamb @Jefffrey Sorry for not responding earlier. I will have some head room this week. And will address the comments.

@Jefffrey The implicit extension is anything but ideal. Looking into the issue, you will find a discussion around approaches. We probably would have to change the signature to have the more ideal scenario.

I will update my findings this evening (CET).
From what I remember we would have to change the signature of the writer or introduce a deprecation for DataFrameWriterOptions.

[hknlof]

This way of implicitly encoding single fileness via adding an extension to the path does seem quite hacky; not sure if this is the correct approach to follow 🤔

@Jefffrey Agreed. Where would you suggest to implement it to have a cleaner version? Maybe setting an option in parquet::ParquetFormatFactory?

[Jefffrey]

This way of implicitly encoding single fileness via adding an extension to the path does seem quite hacky; not sure if this is the correct approach to follow 🤔

@Jefffrey Agreed. Where would you suggest to implement it to have a cleaner version? Maybe setting an option in parquet::ParquetFormatFactory?

I'll try find some time to look into this

[Jefffrey]

Sorry for the long delay, only got around to looking into this issue a bit more recently. Can find my comment here: #13323 (comment)

cc @hknlof

[Jefffrey]

Superseded by #19931