Disable Parallel Parquet Writer by Default, Improve Writing Test Coverage #8854
Conversation
| /// in each row group in each output file are serialized in parallel | ||
| /// leveraging a maximum possible core count of n_files*n_row_groups*n_columns. | ||
| pub allow_single_file_parallelism: bool, default = true | ||
| pub allow_single_file_parallelism: bool, default = false |
|
|
||
| query ?? | ||
| COPY | ||
| (values (struct ('foo', (struct ('foo', make_array(1,2,3)))), make_array(timestamp '2023-01-01 01:00:01',timestamp '2023-01-01 01:00:01')), |
There was a problem hiding this comment.
Thank you. I left a comment there
Would it be possible to change the constant values of the two rows so it is clear they are different
e.g.
| (values (struct ('foo', (struct ('foo', make_array(1,2,3)))), make_array(timestamp '2023-01-01 01:00:01',timestamp '2023-01-01 01:00:01')), | |
| (values (struct ('foo', (struct ('foo', make_array(10,20,30)))), make_array(timestamp '2024-01-01 01:00:01',timestamp '2024-01-01 01:00:01')), |
There was a problem hiding this comment.
I pushed up a change to make the rows distinct and add an array of struct as #8867 is fixed after pulling main.
alamb
left a comment
There was a problem hiding this comment.
Thanks @devinjdangelo -- adding tests is great and very much appreciated.
I think we can merge this PR as is, but if we can improve the tests slightly before doing so that would be great.
|
|
||
| query ?? | ||
| COPY | ||
| (values (struct ('foo', (struct ('foo', make_array(1,2,3)))), make_array(timestamp '2023-01-01 01:00:01',timestamp '2023-01-01 01:00:01')), |
There was a problem hiding this comment.
Thank you. I left a comment there
Would it be possible to change the constant values of the two rows so it is clear they are different
e.g.
| (values (struct ('foo', (struct ('foo', make_array(1,2,3)))), make_array(timestamp '2023-01-01 01:00:01',timestamp '2023-01-01 01:00:01')), | |
| (values (struct ('foo', (struct ('foo', make_array(10,20,30)))), make_array(timestamp '2024-01-01 01:00:01',timestamp '2024-01-01 01:00:01')), |
alamb
left a comment
There was a problem hiding this comment.
Thanks again @devinjdangelo
Which issue does this PR close?
Related to #8853 and #8851
Rationale for this change
A number of issues were recently brought up related to the parallel parquet writer. Most appear to be related to how nested types are being handled. It is not immediately obvious where the flaw is related to handling nested types.
What changes are included in this PR?
Disables parallel writing by default. Adds tests to
copy.sltthat cover the failure cases.Are these changes tested?
Yes
Are there any user-facing changes?
Potentially fewer cores will be used by default when writing parquet files.