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-52. Create ORC InputFormat and OutputFormat implementations for org.apache.hadoop.mapreduce API #27

Closed
wants to merge 3 commits into from

Conversation

omalley
Copy link
Contributor

@omalley omalley commented May 24, 2016

No description provided.

@omalley omalley force-pushed the orc-52 branch 4 times, most recently from 78de632 to d64d724 Compare May 28, 2016 17:05
Reader.Options options = new Reader.Options()
.range(split.getStart(), split.getLength())
.useZeroCopy(OrcConf.USE_ZEROCOPY.getBoolean(conf))
.skipCorruptRecords(OrcConf.SKIP_CORRUPT_DATA.getBoolean(conf));

Choose a reason for hiding this comment

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

These two lines seem to duplicate logic in RecordReaderImpl:

zeroCopy = OrcConf.USE_ZEROCOPY.getBoolean(fileReader.conf);
. As far as I can see that's the only reason conf is passed at all. It could be cleaner if Reader.Options's constructor pulled the requisite values from the conf and only stored/exposed those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, let me figure out how to merge the common code.

@wagnermarkd
Copy link

LGTM. The new names are helpful.

@omalley omalley force-pushed the orc-52 branch 3 times, most recently from 8f0fb18 to ad16a8e Compare June 2, 2016 00:51
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

2 participants