Introduce way to customize prefix of multi file outputs

[DoumanAsh]

Which issue does this PR close?

• Closes Add way to specify custom prefix for partitioned file outputs #19261

Rationale for this change

As per issue, this is most simple approach to allow user to have control over file outputs when writing partitioned parquet/csv

I'm not certain if it would be useful for part-{idx} or not as I do not understand code base well enough to see context of where it is used (for my part I'm mostly interested in making sure randomised file names have unique prefix that I can use to identify these files)

What changes are included in this PR?

Introduces new option partitioned_file_prefix_name within ExecutionOptions with default value empty to retain current behavior by default

This option is used to generate prefix of the file name in writes of datasource' and datasource-* crates

Are these changes tested?

I included basic test to illustrate behaviour of partitioned file output

Are there any user-facing changes?

These changes do not change existing behaviour

[ethan-tyler]

@DoumanAsh Thanks for adding this - being able to prefix output filenames has come up before and this is a clean approach.

Found one bug and a test gap:

Bug: The format string arguments in plan_to_parquet, plan_to_csv, and plan_to_json are reversed. With an empty prefix this produces paths like /part-0.ext (leading slash, no separator before part-). With a prefix like "foo" it produces foo/part-0.ext (prefix becomes a directory). See inline comments for the fix.

Test gap: The test uses with_partition_by() which goes through demux.rs (that code is correct), but doesn't exercise the plan_to_* functions where the bug is. A test using SessionContext::write_parquet or DataFrame::write_parquet without partitioning would catch this.

The demux.rs changes look good.

Let me know if you want me to put together a test case for the other code path.

[DoumanAsh]

@ethan-tyler Thank you for review!
I actually wanted to ask when code paths with part-{idx} are invoked because as newbie to the code base it was not obvious to me
Is there way to control how datafusion decides to split output?

[DoumanAsh]

Ah, nevermind my question, I didn't understand original behavior of parquet writer as I always used with_single_file_output(true) when not partitioning!
I will add tests and verify my code is correct before asking for review

[DoumanAsh]

@ethan-tyler I figured out how to make datasource-* plans to work and added comprehensive tests to cover all possible scenarios where prefix would be used I think
Please take a look when you have time

[DoumanAsh]

Convenient for test code, but generally there seems to be nothing wrong with having it clonable?

[ethan-tyler]

Agreed, nothing wrong with it. It's a plain data struct with no resources or
invariants that cloning would violate. Makes the builder pattern nicer to
work with too.

[ethan-tyler]

LGTM. The named format parameters are cleaner than the helper function
I suggested.

Tests cover all three write paths.

One small thing: the configs.md entry has false in the default column but
it should be empty string. Not blocking, can be fixed in a follow-up.

Thanks for working through all the feedback.

[DoumanAsh]

Squashed commits, rebased on latest master branch

[alamb]

I kicked off the CI checks

[alamb]

Looks like there are some ci failures. Marking as draft

[DoumanAsh]

Just for reference I'm unable to run cargo test in root of repository since substrait build is broken out of box so I only run tests that are affecting functionality.
There seems to be some hidden magic to run checks against config.md so I tried my best to update affected files

[DoumanAsh]

I was able to find out hidden dependencies of subtrait and verified all tests are passing