DuckDB backend

[LaurentRDC]

Fixes #779

[LaurentRDC]

@kmicklas feel free to push to this branch! I'm not going to have time to get to it for at least one week

[tathougies]

Wow this is amazing! So glad to see beam grow like this

[LaurentRDC]

Although I was chatting so much at Amerihac that I ended up doing very little... It'll get done nights and weekends

[LaurentRDC]

At this stage, the backend is functional with the SQL92 standard implemented.

What remains to be built before I consider merging this PR:

• read_parquet;
• iceberg_scan;
• read_csv;

I'm leaving most DuckDB extensions as future work, as well as more recent SQL standards such as SQL99 and SQL2003.

[kmicklas]

I'll take a look at this later today!

[kmicklas]

Is there a reason for having separate entity types for each data source? I can't think of any extra power this would provide. On the other hand, with a single entity type you could in theory do more dynamic behavior like choose at runtime whether to read from CSV or parquet with the same schema.

[kmicklas]

I don't see anything obviously wrong, but starting with this paragraph GitHub started highlighting the markdown as if it were Haskell, which makes me suspicious that there is something wrong with the code block above. (Evil Unicode?)

However the rendered file looks fine, so maybe just a GitHub bug.

[LaurentRDC]

Very strange. I tried removing the csv block, which I wasn't sure if it was supported notation, but it changed nothing. My guess is that's a Github Markdown syntax highlighting quirk

[kmicklas]

Is it possible to use placeholders for paths? This seems like a possible injection mechanism.

[LaurentRDC]

That is a great question. It looks like read_csv and functions like it don't support placeholders. As far as I understand, this means that the filepath or glob cannot be used for injection

[LaurentRDC]

@kmicklas

Is there a reason for having separate entity types for each data source? I can't think of any extra power this would provide. On the other hand, with a single entity type you could in theory do more dynamic behavior like choose at runtime whether to read from CSV or parquet with the same schema.

The reason I went with separate entity types is twofold:

• Different entity types support different input sources, e.g. Parquet and CSV support multiple files, while Iceberg tables support a single FilePath as input;
• Different entity types have different set of options, e.g. CSVOptions and IcebergTableOptions.

The alternative here is that we could have an entity like:

data Parquet
data CSV
data Iceberg

data SourceEntity (fmt :: Type) (table :: (Type -> Type) -> Type)

class IsDuckDBSource fmt where
    type Input fmt -- e.g. `FilePath` or `NonEmpty FilePath`
    type Options fmt -- e.g. `CSVOptions` or `IcebergTableOptions`

I found the mechanism above to be a little heavy to save on boilerplate. Did you have some other mechanism in mind?

[LaurentRDC]

Well, there's a simpler design actually that removes a lot of duplicated code. I suspect this is what Ken was suggesting:

data DataSourceEntity (table :: (Type -> Type) -> Type)

data DataSource
    = CSV (NonEmpty FilePath) CSVOptions
    | Parquet (NonEmptyFilePath) -- no parquet options
    | Iceberg FilePath IcebergOptions

dataSource :: DataSource 
           -> EntityModification (DatabaseEntity DuckDB db) DuckDB (DataSource table)

[kmicklas]

Well, there's a simpler design actually that removes a lot of duplicated code. I suspect this is what Ken was suggesting:

data DataSourceEntity (table :: (Type -> Type) -> Type)

data DataSource
    = CSV (NonEmpty FilePath) CSVOptions
    | Parquet (NonEmptyFilePath) -- no parquet options
    | Iceberg FilePath IcebergOptions

dataSource :: DataSource 
           -> EntityModification (DatabaseEntity DuckDB db) DuckDB (DataSource table)

Yup! This is exactly what I meant.

My only remaining concern is that I think we should document more clearly the injection possibility with untrusted paths.

[LaurentRDC]

My only remaining concern is that I think we should document more clearly the injection possibility with untrusted paths.

Done. Thanks for the review!