You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "djspiewak (via GitHub)" <gi...@apache.org> on 2025/05/21 09:13:42 UTC
[PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
djspiewak opened a new pull request, #46928:
URL: https://github.com/apache/spark/pull/46928
This PR fixes an issue in the vectorized parquet reader with respect to executing the `explode` function on nested arrays where the array cuts across two or more pages. It's probably possible to minimize this slightly more but I wasn't able to find a reproducer. It's also worth noting that this issue illustrates a current gap in the lower-level unit tests for the vectorized reader, which don't appear to test much related to output vector offsets.
The bug in question was a simple typo: the output row offset was used to dereference nested array lengths rather than input row offset. This only matters for the explode function and then only when resuming the same operation on a second page. This case (and all related cases) are, at present, untested. I added a high-level test and example `.parquet` file which reproduces the issue and verifies the fix, but it would be ideal if more tests were added at a lower level. It is very likely that other similar bugs are present within the vectorized reader as it relates to nested substructures remapped during the query pipeline.
### What changes were proposed in this pull request?
It's a fairly straightforward typo issue in the code.
### Why are the changes needed?
The vectorized parquet reader does not correctly handle this case
### Does this PR introduce _any_ user-facing change?
Aside from fixing the vectorized reader? No.
### How was this patch tested?
Unit test (well, more of an integration test) included in PR
### Was this patch authored or co-authored using generative AI tooling?
Nope
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2165085691
Should we have a corresponding JIRA ticket for this fix?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-3181628534
Hi, @djspiewak and @sunchao . It seems that we didn't deliver this to `branch-4.0`.
> Merged to master/branch-3.5, thanks!
I'm trying to deliver this via Apache Spark 4.0.1. I'll backport this.
Please let me know if there is any issue.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #46928: Corrected row index usage when exploding packed arrays in vectorized reader
URL: https://github.com/apache/spark/pull/46928
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao closed pull request #46928: [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader
URL: https://github.com/apache/spark/pull/46928
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2902811237
I've retargeted this pull request and updated the commits. I also took a moment to reexamine the test to see what can be done about the binary file. It's important to understand that this bug manifests *specifically* when exploding across a row group boundary. That's doable without encoding the test data as parquet, but it's not particularly *easy* or obvious to do. I'd also like to reiterate that most of the test suite I modified is using checked in parquet files.
Again, happy to change this if there's already a pre-existing pattern for how to write this sort of test (or intrinsically generate this sort of data), but given the nature of the bug, the present diff seems optimal to me.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2231694299
Is this being held up by anything? Any JIRA would be a fairly trivial transliteration of the test case that I added. Note the query and the example parquet file. That example does not work today on production Databricks instances (resulting an `ArrayIndexOutOfBoundsException`).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-3181636079
This is back ported via the following commit.
- dbbd91c7b3c
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2902832941
cc @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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2902832359
Apache Spark repository takes advantage of the GitHub resource of contributor fork to avoid the lack of resources. Could you setup GitHub action on your repository?
- https://spark.apache.org/contributing.html
```
Ref: bug/packed-list-vectorized
SHA: 466266f25cbf86ef5e810344754aba2af7e61c80
Error: There was a new unsynced commit pushed. Please retrigger the workflow.
at eval (eval at callAsyncFunction (/home/runner/work/_actions/actions/github-script/v6/dist/index.js:15143:16), <anonymous>:74:11)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async main (/home/runner/work/_actions/actions/github-script/v6/dist/index.js:15236:20)
Error: Unhandled error: Error: There was a new unsynced commit pushed. Please retrigger the workflow.
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #46928: Corrected row index usage when exploding packed arrays in vectorized reader
URL: https://github.com/apache/spark/pull/46928
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2901448498
@LuciferYang Slightly different situation here as jars and class files are not the same as parquet. Is there an existing example of how to generate *test* parquet files? As I noted, there are already a significant number of such files so I assume this has been discussed before.
If nothing has been standardized I can work on sorting that out just trying to avoid redundancy here.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2905111016
@dongjoon-hyun I've reenabled actions. Please let me know if I need to reconfigure anything else on my end
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2899739665
This is a fair stance, though I'll note that the repository already has many violations of this so far as I can tell. If this is considered a blocker to merge I can work on generating the test data automatically.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2907950534
Thank you all.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2907939404
Merged to master/branch-3.5, thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2897258132
@djspiewak How does `packed-list-vectorized.parquet` generate, and could it be generated on the fly? BTW, I suppose the bug also existed on the master branch, the fix should target the master instead of branch-3.5
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2899001765
It was generated using an internal tool written in Go using standard libraries. It took a fair bit of minimization of different test files that were breaking Databricks to narrow it down to this one, which is I believe minimal.
It's definitely possible to generate this situation dynamically but that seemed considerably harder than just baking in the reproducer in this fashion.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2903473851
> I've retargeted this pull request and updated the commits. I also took a moment to reexamine the test to see what can be done about the binary file. It's important to understand that this bug manifests _specifically_ when exploding across a row group boundary. That's doable without encoding the test data as parquet, but it's not particularly _easy_ or obvious to do. I'd also like to reiterate that most of the test suite I modified is using checked in parquet files.
>
> Again, happy to change this if there's already a pre-existing pattern for how to write this sort of test (or intrinsically generate this sort of data), but given the nature of the bug, the present diff seems optimal to me.
If there are no objections from other Spark PMCs, I accept this testing proposal.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2905101702
Oh lol I actively *disabled* the actions on my fork. I'll sort that out
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2899723762
@djspiewak the ASF projects highly discourage including binary files(e.g. class files, jars, data files that can not be edited via a text editor) in the source release/codebase
There is a recent discussion about removing testing jars from the codebase
https://lists.apache.org/thread/0ro5yn6lbbpmvmqp2px3s2pf7cwljlc4
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52240] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2905118111
@djspiewak Could you please rebase this one? I manually restarted the GitHub Action jobs, but it still failed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2899934288
@djspiewak Indeed, it is clear that there are relevant counterexamples in the current Spark repository. However, the Spark developers have taken proactive measures to address this issue, as evidenced by the following pr:
- https://github.com/apache/spark/pull/50378
- https://github.com/apache/spark/pull/50422
- https://github.com/apache/spark/pull/50790
I believe that this problem will be completely eradicated in the future. Therefore, it would be preferable if you could optimize the testing process by generating the necessary Parquet files as test cases during the testing phase. Thanks ~
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2614656241
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Corrected row index usage when exploding packed arrays in vectorized reader [spark]
Posted by "djspiewak (via GitHub)" <gi...@apache.org>.
djspiewak commented on PR #46928:
URL: https://github.com/apache/spark/pull/46928#issuecomment-2616141314
Is there any particular reason this is languishing? I'd like to make sure this gets fixed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org