Repository navigation
Port tests in parquet_schema.rs to sqllogictest #8207
Description
Is your feature request related to a problem or challenge?
Part of #6195. We are trying to
- keep the DataFusion code base clean and easy to contribute to and modify
- Understand what features are covered and what are not
Part of doing so is having a single location for most test coverage so new tests can be added easily and people can follow the existing patterns easily
Describe the solution you'd like
Port the tests in https://github.com/apache/arrow-datafusion/blob/main/datafusion/core/tests/sql/parquet_schema.rs to
parquet_schema.slt in https://github.com/apache/arrow-datafusion/tree/main/datafusion/sqllogictest/test_files
Note you can create catalogs and schema via SQL now
Catalogs: https://arrow.apache.org/datafusion/user-guide/sql/ddl.html#create-database
Schema: https://arrow.apache.org/datafusion/user-guide/sql/ddl.html#create-schema
Notes:
- Instructions for running sqllogitest are here: https://github.com/apache/arrow-datafusion/tree/main/datafusion/sqllogictest
Describe alternatives you've considered
No response
Additional context
I think these are good first issues as they teach the contributor about the DataFusion codebase and tests, as well as see the end user apis in practice.
Activity
matthewmturner commentedon Dec 22, 2023
ill take this
matthewmturner commentedon Dec 23, 2023
@alamb I could be wrong (I'm still getting used to the code base again) - but it looks like porting these tests may require adding the ability to set whether to read metadata or not from SQL (i.e. add
WITH METADATAsimilar toWITH HEADER ROW). From what ive seen choosing whether to read metadata is only possible by explicitly settingParquetReadOptionswithctx.read_parquet. If you agree I can create an issue for that and work on it before coming back to this.alamb commentedon Dec 24, 2023
I think you are correct that this test is not really a SQL level test (as we can't control the metadata reading) @matthewmturner
Porting this to SQL doesn't make sense. I think a better thing would be to move the test into the "parquet" tests as it is really a test of the parquet reader. I proposed doing so in #8644
matthewmturner commentedon Dec 24, 2023
Got it - would you just be able to expand on why it doesnt make sense to add the
WITH METADATAas a an option from SQL?alamb commentedon Dec 26, 2023
In my mind it is because the usecase for preserving the metadata is when DataFusion is used as a library to build other systems (which may interpret the schema level metadata) and in which case the user is likely to be using lower level APIs than SQL.
That being said, if anyone has a use case for controlling schema metadata output at the SQL level it would make sense to add it to the SQL interface.