Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRILL-6447: Fixed a sanity check condition #1291

Closed
wants to merge 1 commit into from

Conversation

sachouche
Copy link
Contributor

@sachouche sachouche commented May 25, 2018

The Parquet bulk reader asserted that resetting a Page Definition Level should be performed only for a Flat Schema; the check should have tested for a value of zero or one.

@sachouche
Copy link
Contributor Author

@arina-ielchiieva can you please review this fix?

Thanks!

@@ -445,8 +445,8 @@ public void clear(){
* @throws IOException An IO related condition
*/
void resetDefinitionLevelReader(int skipCount) throws IOException {
if (parentColumnReader.columnDescriptor.getMaxDefinitionLevel() != 0) {
throw new UnsupportedOperationException("Unsupoorted Operation");
if (parentColumnReader.columnDescriptor.getMaxDefinitionLevel() > 1) {
Copy link
Member

@vrozov vrozov May 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already part of Parquet version upgrade (see PR #1259), no need to fix it.

@priteshm
Copy link

@sachouche can we add the JIRA number in the title as well?

@sachouche sachouche changed the title Fixed a sanity check condition DRILL-6447: Fixed a sanity check condition May 29, 2018
@arina-ielchiieva
Copy link
Member

@sachouche any update based to Vlad's feedback?

@vrozov
Copy link
Member

vrozov commented Jun 11, 2018

@arina-ielchiieva We agreed that this PR can be closed and I'll add fix for VarLenBulkPageReader.java to the parquet upgrade PR.

@arina-ielchiieva
Copy link
Member

arina-ielchiieva commented Jun 11, 2018

Sounds good. Closing the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants