Fix bugs related to partial aggregation pushdown refactoring#22131
Fix bugs related to partial aggregation pushdown refactoring#22131rschlussel merged 2 commits intoprestodb:masterfrom
Conversation
accidentally used the orc exception handling during refactoring, as the Parquet file never got added
The OrcDataSource wasn't being closed unless there was an error.
| { | ||
| private ParquetPageSourceFactoryUtils() {} | ||
|
|
||
| public static PrestoException mapToPrestoException(Exception e, Path path, HiveFileSplit fileSplit) |
There was a problem hiding this comment.
this was copied wholesale from the code that had been in ParquetPageSourceFactory. No changes here.
There was a problem hiding this comment.
We can probably generalize common exceptions and keep them in a all purpose PageSourceFactoryUtils
There was a problem hiding this comment.
To clarify this is the common area. This code was copied from what had previously been in ParquetPageSourceFactory. I deleted it from there in my last PR to move it to this common utility class, but accidentally didn't include this file when I pushed the changes in my last PR and didn't notice because I also had the wrong import in the code.
| { | ||
| private ParquetPageSourceFactoryUtils() {} | ||
|
|
||
| public static PrestoException mapToPrestoException(Exception e, Path path, HiveFileSplit fileSplit) |
There was a problem hiding this comment.
We can probably generalize common exceptions and keep them in a all purpose PageSourceFactoryUtils
presto-hive/src/main/java/com/facebook/presto/hive/orc/OrcAggregatedPageSourceFactory.java
Show resolved
Hide resolved
| import static com.facebook.presto.hive.orc.OrcPageSourceFactoryUtils.mapToPrestoException; | ||
| import static com.facebook.presto.hive.parquet.HdfsParquetDataSource.buildHdfsParquetDataSource; | ||
| import static com.facebook.presto.hive.parquet.ParquetPageSourceFactory.createDecryptor; | ||
| import static com.facebook.presto.hive.parquet.ParquetPageSourceFactoryUtils.mapToPrestoException; |
ajaygeorge
left a comment
ajaygeorge
left a comment
There was a problem hiding this comment.
Fix parquet exception handling also looks good.
3 participants
Description
Fixes bugs from the refactoring of error handling for partial aggregation pushdown #22011.
Impact
Error messages should now match what they were before #22011, and there should no longer be a resource leak.
Test Plan
CI
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.