datafusion icon indicating copy to clipboard operation
datafusion copied to clipboard

Add `FileScanConfig::new()` API

Open alamb opened this issue 1 year ago • 1 comments

Which issue does this PR close?

Part of https://github.com/apache/datafusion/issues/10546

Rationale for this change

While working on https://github.com/apache/datafusion/pull/10549 it was cumbersome to create a FileScanConfig -- there are many different fields, many of which need default values which require some thought to figure out.

The number of fields also makes it harder to see what is specifically set and what is the default

This has bothered me for a long time, so I made a PR to fix it.

You can see this in action here https://github.com/apache/datafusion/pull/10618

What changes are included in this PR?

  1. Add FileScanConfig::new() that takes required information
  2. Add an example
  3. Add builder style APIs for updating other fields
  4. Update the code to use the new API

Are these changes tested?

Covered by existing tests

Are there any user-facing changes?

Nicer and easier to use API for creating FileScanConfig

alamb avatar May 22 '24 15:05 alamb

Thank you @phillipleblanc

Is that something you generally do for all builder-type APIs, or it depends?

I would say it depends. In this case I hadd a use case for add style in the example I am working on for indexing (you can see this in action here https://github.com/apache/datafusion/pull/10618)

However, I can see how we could do something else in this case, so perhaps I should remove the set style APIs 🤔

alamb avatar May 23 '24 10:05 alamb

Thank you @metegenez

alamb avatar May 24 '24 21:05 alamb

Thanks everyone for the reviews!

alamb avatar May 25 '24 11:05 alamb