zahed1994 commented on PR #58176: URL: https://github.com/apache/spark/pull/58176#issuecomment-5493961744
Hi @sunchao, thanks for catching the `DELTA_BYTE_ARRAY` correctness regression in #54805. I’ve reopened #58176 to continue working on SPARK-55968 and have revised the approach based on your feedback. The current patch introduces a dedicated `VectorizedReaderCapacityOverflowException` for the vectorized reader capacity-exhaustion path, so `DataSourceUtils.shouldIgnoreCorruptFileException` can distinguish this condition by exception type rather than by matching the exception message. However, your `DELTA_BYTE_ARRAY` example highlights an important remaining distinction: a corrupt encoded length can also reach `WritableColumnVector.reserve()` and produce the same underlying integer-overflow error. In that case, it should continue to be treated as a corrupt file when `ignoreCorruptFiles=true`. I think the remaining fix should therefore be to ensure that invalid capacities originating from encoded input are rejected before they can be classified as `VectorizedReaderCapacityOverflowException`. Concretely, I’m looking at the following behavior: * A valid positive capacity request that exceeds the vectorized reader's supported capacity → `VectorizedReaderCapacityOverflowException` → `ignoreCorruptFiles` does **not** suppress it. * An invalid/negative capacity originating from malformed Parquet data (such as the negative suffix length in the `DELTA_BYTE_ARRAY` case) → a regular corruption-related exception such as `IllegalArgumentException` → `ignoreCorruptFiles` continues to suppress it. * `DataSourceUtils` should only special-case the dedicated capacity-overflow exception and should not rely on matching the exception message. I’ll add a regression test covering the malformed `DELTA_BYTE_ARRAY` case you identified, in addition to the existing capacity-overflow test, so that both behaviors are explicitly covered. This should preserve the original SPARK-55968 fix while avoiding the correctness regression in #54805. Would appreciate your thoughts on whether this separation between invalid input capacity and genuine reader capacity exhaustion is the right approach, @sunchao. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
