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

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning apache/main
    Building datafusion v54.0.0 (current)
       Built [ 104.226s] (current)
     Parsing datafusion v54.0.0 (current)
      Parsed [   0.034s] (current)
    Building datafusion v54.0.0 (baseline)
       Built [ 101.203s] (baseline)
     Parsing datafusion v54.0.0 (baseline)
      Parsed [   0.035s] (baseline)
    Checking datafusion v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.643s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [ 208.121s] datafusion
    Building datafusion-common v54.0.0 (current)
       Built [  33.899s] (current)
     Parsing datafusion-common v54.0.0 (current)
      Parsed [   0.060s] (current)
    Building datafusion-common v54.0.0 (baseline)
       Built [  33.954s] (baseline)
     Parsing datafusion-common v54.0.0 (baseline)
      Parsed [   0.061s] (baseline)
    Checking datafusion-common v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.698s] 223 checks: 222 pass, 1 fail, 0 warn, 30 skip

--- failure constructible_struct_adds_field: externally-constructible struct adds field ---

Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field ExecutionOptions.partitioned_file_prefix_name in /home/runner/work/datafusion/datafusion/datafusion/common/src/config.rs:723

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  70.675s] datafusion-common
    Building datafusion-datasource v54.0.0 (current)
       Built [  37.351s] (current)
     Parsing datafusion-datasource v54.0.0 (current)
      Parsed [   0.032s] (current)
    Building datafusion-datasource v54.0.0 (baseline)
       Built [  37.472s] (baseline)
     Parsing datafusion-datasource v54.0.0 (baseline)
      Parsed [   0.033s] (baseline)
    Checking datafusion-datasource v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.260s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  76.188s] datafusion-datasource
    Building datafusion-datasource-csv v54.0.0 (current)
       Built [  37.994s] (current)
     Parsing datafusion-datasource-csv v54.0.0 (current)
      Parsed [   0.011s] (current)
    Building datafusion-datasource-csv v54.0.0 (baseline)
       Built [  37.976s] (baseline)
     Parsing datafusion-datasource-csv v54.0.0 (baseline)
      Parsed [   0.011s] (baseline)
    Checking datafusion-datasource-csv v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.101s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  77.168s] datafusion-datasource-csv
    Building datafusion-datasource-json v54.0.0 (current)
       Built [  38.188s] (current)
     Parsing datafusion-datasource-json v54.0.0 (current)
      Parsed [   0.012s] (current)
    Building datafusion-datasource-json v54.0.0 (baseline)
       Built [  38.045s] (baseline)
     Parsing datafusion-datasource-json v54.0.0 (baseline)
      Parsed [   0.013s] (baseline)
    Checking datafusion-datasource-json v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.092s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  77.389s] datafusion-datasource-json
    Building datafusion-datasource-parquet v54.0.0 (current)
       Built [  43.346s] (current)
     Parsing datafusion-datasource-parquet v54.0.0 (current)
      Parsed [   0.030s] (current)
    Building datafusion-datasource-parquet v54.0.0 (baseline)
       Built [  43.403s] (baseline)
     Parsing datafusion-datasource-parquet v54.0.0 (baseline)
      Parsed [   0.031s] (baseline)
    Checking datafusion-datasource-parquet v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.158s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  87.989s] datafusion-datasource-parquet
    Building datafusion-sqllogictest v54.0.0 (current)
       Built [ 177.671s] (current)
     Parsing datafusion-sqllogictest v54.0.0 (current)
      Parsed [   0.022s] (current)
    Building datafusion-sqllogictest v54.0.0 (baseline)
       Built [ 177.151s] (baseline)
     Parsing datafusion-sqllogictest v54.0.0 (baseline)
      Parsed [   0.023s] (baseline)
    Checking datafusion-sqllogictest v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.088s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [ 358.087s] datafusion-sqllogictest