MultiFileReader refactor

[samansmink]

This PR is the first step of a refactor of the MultiFileReader.

Goals

The goals of the refactor as a whole are:

• allow late expansion of a list of file(globs)
• allow switching MultiFileReaders of existing table functions (to allow reusing TableFunctions through catalog from in an extension)

Current problem

To illustrate why this is important lets look at an example:

SELECT * FROM "s3://bucket/**/*.parquet"

What we currently do for the above query is to fully expand the glob in the bind phase. This can be very slow if the bucket is large as it has several problems:

• bind phase is single threaded, meaning that this process is fully sequential
• we cannot reduce the amount of file listing to be done with limits or filter pushdown
• this does not allow extensions to implement more clever ways to scan big sets of files

This PR

Because the MultiFileReader code is already quite complex, and is used by some pretty complex parts of duckdb (the CSV reader mainly). The refactor will be done in steps. In this first step I aim to achieve:

• Allow subclassing the MultiFileReader and injecting a factory method for it in a TableFunction
• Add a MultiFileList interface
• Add a SimpleMultiFileList class that implements the MultiFileList interface but still being a fully expanded list of files internally.
• Switch from using a vector<string> to using the new MultiFileList class in most MultiFileReader code
• Switch the Parquet Scan to use the MultiFileList in a way that shows how the MultiFileList interface enables interacting with a lazily generated file list
• Add virtual methods for Filter Pushdown to MultiFileList and MultiFileReader
• Adds a new (unused) bind method to the MultiFileReader that can be used by subclasses to implement custom binding for a multifileread

Note that functionally nothing should have changed in DuckDB with this PR

Todo's

• The JSON and CSV reader still store a vector of strings in their bind data, this should switch to using a MultiFileList, but some work regarding serialization and backwards compatibility remains there
• This PR just adds complexity to an already quite complex part of DuckDB, we should investigate where things can be simplified
• I'm currently still missing a way for a custom MultiFileReader to pass state between MultiFileReader::BindOptions and MultiFileReader::FinalizeBind

Notable points to review

• The logic in ParquetScanInitGlobal has changed and is non trivial to say the least
• I have some assert(false) in ParquetScanInitGlobal where we may want to just throw internalexceptions?
• The result of making the MultiFileReader methods non-static is that I have some weird empty MultiFileReader objects in several places now, the idea being that those place should make sure they should properly initialize a multifilereader at some point and reuse the same one.

[Tishj]

This is case sensitive, doesn't FILE_ROW_NUMBER break in the same way?
I understand this is existing logic that is merely moved, but still this seems problematic

[Mytherin]

Thanks for the PR! Looks good - some comments below:

[Mytherin]

Thanks for the fixes! Some more comments:

[Mytherin]

Can we turn this into a regular mutex?

[samansmink]

vector doesn't like that: mutex is not movable.

The idea is that with this you can:

• get a ref to this mutex
• drop the global state lock
• grab the file lock
• release the file lock
• regrab the global state lock

with the idea of opening a new file without locking global state while doing so: in the meantime the vector holding the mutex may get resized.

[Mytherin]

Nightly test run here - https://github.com/samansmink/duckdb/actions/runs/8908217982 - PTAL at the threadsan

[samansmink]

@Mytherin PTAL

I have significantly reworked the MultiFileList to make it thread-safe. Some notes:

• Subclasses of the MultiFileList now implement locking (if necessary)
• The SimpleMultiFileList is completely constant, so it can be lock-free
• The ComplexFilterPushdown method now returns a new list instead of modifying the existing one in-place. This was the crux to make this work nicely: MultiFileLists are now effectively immutable even though internally they can expand lazily.

This solves basically all problems: Multiple scans can reuse the same MultiFileLists without problems, and we can just leave the multifilelist in the BindData (even honoring the fact that the bind_data are const to some degree, as the list only changes its internal state, its result should be consistent even under parallel use).

Currently what this means is that when a filter is pushed down into a MultiFileReader AND hive partitioning or filename is enabled, the GlobMultiFileList gets transformed into a SimpleMultiFileList during filter pushdown. In the future we can:

• add lazy glob expansion
• add filter pushdown into the glob

If you think this is good and threadsan has no objections, I think this is good to go.

I will update the PR description with an explanation of how to use the MultiFileList for future reference once this is approved.

[Mytherin]

Sounds great!

[samansmink]

Nightly run started here for threadsan: https://github.com/samansmink/duckdb/actions/runs/8930201930, ThreadSan seems happy with the changes

[Mytherin]

Thanks! Looks good