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

ORC-313,ORC-317: Check types in footer #225

Closed
wants to merge 4 commits into from

Conversation

stiga-huang
Copy link
Contributor

We need to verify the subtype count of LIST and MAP types. Otherwise,
the ill types will lead the c++ reader to crash.

The java reader also has this problem.

@stiga-huang
Copy link
Contributor Author

With the bug of ORC-317, I think we can check the protobuf Types at the end of orc::readFooter for

  • ORC-313: check the subtype count of LIST and MAP
  • ORC-317: check that the type tree is valid (not containing loop)

Will update this PR later.

@stiga-huang stiga-huang changed the title ORC-313: Check subtype count in converting protobuf Types ORC-313,ORC-317: Check types in footer Mar 8, 2018
We need to verify the subtype count of LIST and MAP types. Otherwise,
the ill types will lead the c++ reader to crash.

The java reader also has this problem.
@stiga-huang
Copy link
Contributor Author

Hi @omalley @majetideepak, this PR is ready for review. Could you have a look?

@majetideepak
Copy link
Contributor

@stiga-huang can you open a separate PR or each JIRA? Thanks.

@stiga-huang
Copy link
Contributor Author

@majetideepak OK. When I separate the PR, I found orc-test failed in TestWriter.writeTimestamp (ORC-320). I might need to fix that first.

@stiga-huang
Copy link
Contributor Author

@majetideepak I opened #229 for ORC-313

@majetideepak
Copy link
Contributor

@stiga-huang I will look into these this week. Can you fix the title of this PR? Thanks!

@stiga-huang
Copy link
Contributor Author

@majetideepak To be clean, I open #231 for ORC-317. I'd like to close this PR. Thanks for your review!

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.

2 participants