This is an automated comment for commit 72dc186 with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🔴 failure

I'd mark closes #23297

I found on my re-implemented parquet reader that filters need to be pushed down to column decoder, especially for dictionary-encoded string columns.

@al13n321 Clickhouse Arrow version lags two big versions of upstream, do you have plan to upgrade the version?

I found on my re-implemented parquet reader that filters need to be pushed down to column decoder, especially for dictionary-encoded string columns.

I don't understand, can you elaborate?

This PR just looks at column chunk statistics in the FileMetaData struct, then just skips row groups. No deeper integration with the decoder seems necessary. Do you mean one of these?:

• We could also filter out individual pages based on page statistics. But that seems really difficult and not worthwhile.
• Don't expand dictionary-encoded columns into full columns, keep them in memory as dictionary-encoded (ColumnLowCardinality) throughout the pipeline as much as possible. Kind of like if LowCardinality wasn't a type but just an internal detail of column representation. That would be nice. Seems unrelated to this task.
• We could do PREWHERE on row groups, just like on merge tree granules. I.e. first decode the columns needed for PREWHERE, then evaluate the PREWHERE expression, and if all rows are rejected don't read+decode the rest of the columns. Can also skip individual pages based on the filter mask.

Btw, how's the reimplementation going? I'm looking forward to it!

I found on my re-implemented parquet reader that filters need to be pushed down to column decoder, especially for dictionary-encoded string columns.

I don't understand, can you elaborate?

This PR just looks at column chunk statistics in the FileMetaData struct, then just skips row groups. No deeper integration with the decoder seems necessary. Do you mean one of these?:

• We could also filter out individual pages based on page statistics. But that seems really difficult and not worthwhile.
• Don't expand dictionary-encoded columns into full columns, keep them in memory as dictionary-encoded (ColumnLowCardinality) throughout the pipeline as much as possible. Kind of like if LowCardinality wasn't a type but just an internal detail of column representation. That would be nice. Seems unrelated to this task.
• We could do PREWHERE on row groups, just like on merge tree granules. I.e. first decode the columns needed for PREWHERE, then evaluate the PREWHERE expression, and if all rows are rejected don't read+decode the rest of the columns. Can also skip individual pages based on the filter mask.

Btw, how's the reimplementation going? I'm looking forward to it!

Just mentioning. It doesn't mean that this feature is bad.
The second point. Filters act on the encoded value, which is filtered and then decoded. I have implemented this method, but the performance is still not as good as the Velox Parquet reader, and it is still being investigated. The main gap here lies in the processing of the filter, and there is no gap in the performance of data decoding. There's an experimental PR here, and the code is ugly. https://github.com/oap-project/gluten/pull/1997/files

We could do PREWHERE on row groups, just like on merge tree granules. I.e. first decode the columns needed for PREWHERE, then evaluate the PREWHERE expression, and if all rows are rejected don't read+decode the rest of the columns. Can also skip individual pages based on the filter mask.

IIUC then prewhere would not take effect on parquet pushdown, and would have to manually be implemented with nested subqueries in the current form of this PR?

IIUC then prewhere would not take effect on parquet pushdown, and would have to manually be implemented with nested subqueries in the current form of this PR?

Yes.

Can we add a performance test? (in tests/performance)

Nice!

Performance Comparison regressions seem spurious, except for this one:

SELECT WatchID FROM table_Parquet FORMAT Null

which now takes 3 ms instead of 2 ms. It's plausible that ~1ms is really how long the new filtering code takes (on a file with 133 columns and 1 row group), and I guess that's ok.