You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "mathfool (via GitHub)" <gi...@apache.org> on 2024/01/18 14:44:19 UTC
[PR] Spark: Fix reading 2 level array issue [iceberg]
mathfool opened a new pull request, #9515:
URL: https://github.com/apache/iceberg/pull/9515
Trying to fix the issue:
https://github.com/apache/iceberg/issues/9497
Basically, the RL and DL were 1 level lower than expected. Add an if/else to avoid impacting 3 level array representation.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "wallacms (via GitHub)" <gi...@apache.org>.
wallacms commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-2607355455
Can we re-open this (and merge it?) I've tested this code and it does fix the problem.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-2607389264
@wallacms probably the best would be if you could bring up this issue on the [DEV mailing list](https://iceberg.apache.org/community/#mailing-lists) to reach the broader community
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#discussion_r1941637351
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/data/SparkParquetReaders.java:
##########
@@ -198,8 +198,16 @@ public ParquetValueReader<?> list(
Types.ListType expectedList, GroupType array, ParquetValueReader<?> elementReader) {
String[] repeatedPath = currentPath();
- int repeatedD = type.getMaxDefinitionLevel(repeatedPath) - 1;
- int repeatedR = type.getMaxRepetitionLevel(repeatedPath) - 1;
+ int repeatedD;
+ int repeatedR;
+
+ if (type.isRepetition(Type.Repetition.REPEATED)) {
+ repeatedD = type.getMaxDefinitionLevel(repeatedPath);
+ repeatedR = type.getMaxRepetitionLevel(repeatedPath);
+ } else {
+ repeatedD = type.getMaxDefinitionLevel(repeatedPath) - 1;
+ repeatedR = type.getMaxRepetitionLevel(repeatedPath) - 1;
+ }
Review Comment:
This would need tests if it were to be merged.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-1898924695
Just FYI, the [spec](https://iceberg.apache.org/spec/#parquet) mentions that lists in Parquet must use the 3-level representation
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "wallacms (via GitHub)" <gi...@apache.org>.
wallacms commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-2634805095
I'm happy to write some tests, but it seemed like it was being rejected / ignored on more fundamental grounds (i.e based on the spec). Would a PR that includes tests be likely to be accepted (assuming tests are adequate, etc)?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "mathfool (via GitHub)" <gi...@apache.org>.
mathfool commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-1899854105
@nastra Thanks for the comments, since the code in the lib suggested that 2-level array is supported, so it should work as expected.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-2408755049
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9515:
URL: https://github.com/apache/iceberg/pull/9515#issuecomment-2424319453
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Fix reading 2 level array issue [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9515: Spark: Fix reading 2 level array issue
URL: https://github.com/apache/iceberg/pull/9515
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org