Skip to content

[SPARK-20682][SPARK-15474][SPARK-21791] Add new ORCFileFormat based on ORC 1.4.1 #19651

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.

Sign up for GitHub

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

Closed
dongjoon-hyun wants to merge 22 commits into from
Closed

[SPARK-20682][SPARK-15474][SPARK-21791] Add new ORCFileFormat based on ORC 1.4.1 #19651

Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b342196
[SPARK-20682][SPARK-15474][SPARK-21791] Add new ORCFileFormat based o…
dongjoon-hyun May 15, 2017
ca78ac7
Address comments.
dongjoon-hyun Nov 7, 2017
9d18834
Recover OrcFileFormat back, avoid function serialization, add TODO.
dongjoon-hyun Nov 10, 2017
6971cdf
Simplify `isSearchableType`
dongjoon-hyun Nov 10, 2017
b373495
Avoid boxing for primitive types.
dongjoon-hyun Nov 10, 2017
de8b509
Use getMissingColumnNames instead of getMissingSchema
dongjoon-hyun Nov 12, 2017
726406f
Move withNullSafe to OrcUtils
dongjoon-hyun Nov 12, 2017
4097457
Handle top-level columns in a while-loop and split the logic for Stru…
dongjoon-hyun Nov 12, 2017
8e0d392
fix
dongjoon-hyun Nov 12, 2017
9e3ac1a
Address comments.
dongjoon-hyun Nov 13, 2017
a3ebfbf
fix
dongjoon-hyun Nov 13, 2017
cc40fba
Move out column name handling logic.
dongjoon-hyun Nov 22, 2017
f482179
Use Updater like Parquet.
dongjoon-hyun Nov 28, 2017
e13dfa3
fix
dongjoon-hyun Nov 29, 2017
fdab6a7
remove outdate comment.
dongjoon-hyun Nov 29, 2017
daef4ba
refactor
cloud-fan Dec 1, 2017
f143e17
address comment
cloud-fan Dec 1, 2017
8a34731
Revert the change on HiveInspectors.scala.
dongjoon-hyun Dec 1, 2017
74cb053
Merge branch 'PR-2' into SPARK-20682
dongjoon-hyun Dec 1, 2017
eae50b3
bug fix
cloud-fan Dec 1, 2017
520837f
Merge pull request #3 from cloud-fan/orc
dongjoon-hyun Dec 2, 2017
71be008
Restore old ORC implementation.
dongjoon-hyun Dec 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Move out column name handling logic.
  • Loading branch information
@dongjoon-hyun
dongjoon-hyun committed Nov 29, 2017
commit cc40fba4e4f12d7f65dd7f426fc20f3b5c2a7b6e
20 changes: 15 additions & 5 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcDeserializer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,19 @@ private[orc] class OrcDeserializer(

private[this] val length = requiredSchema.length

private[this] val unwrappers = requiredSchema.map(_.dataType).map(unwrapperFor).toArray
private[this] val unwrappers = requiredSchema.map { f =>
if (missingColumnNames.contains(f.name)) {
(value: Any, row: InternalRow, ordinal: Int) => row.setNullAt(ordinal)
} else {
unwrapperFor(f.dataType)
}
}.toArray

def deserialize(orcStruct: OrcStruct): InternalRow = {
var i = 0
val names = orcStruct.getSchema.getFieldNames
while (i < length) {
val name = requiredSchema(i).name
val writable = if (missingColumnNames.contains(name)) {
val fieldRefs = requiredSchema.map { f =>
val name = f.name
if (missingColumnNames.contains(name)) {
null
} else {
if (names.contains(name)) {
Copy link
Contributor

@cloud-fan cloud-fan Nov 13, 2017

Choose a reason for hiding this comment

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

Doing this kind of thing in the loop is pretty slow, especially on this critical path. Let's pre-compute it before the loop. Again, PLEASE follow the previous code, which did this check when creating the unwrapper.

Copy link
Contributor

@cloud-fan cloud-fan Nov 20, 2017

Choose a reason for hiding this comment

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

BTW do we really need to handle missing columns for nested fields?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Nov 22, 2017

Choose a reason for hiding this comment

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

  1. I moved out the column-name logic from while, but it still requires OrcStruct because we don't have StructObjectInspector. So, we cannot move out this from iter.map.
  2. This is about top-level fields. Could you tell me what do you mean by nested fields here?

Expand All @@ -56,6 +61,11 @@ private[orc] class OrcDeserializer(
orcStruct.getFieldValue("_col" + dataSchema.fieldIndex(name))
Copy link
Contributor

@cloud-fan cloud-fan Nov 22, 2017

Choose a reason for hiding this comment

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

actually, do we really need missingCols? If the name is missing, orcStruct.getFieldValue will just return null, isn't it?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Nov 29, 2017

Choose a reason for hiding this comment

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

OrcStruct will raise Exception here.

Copy link
Contributor

@cloud-fan cloud-fan Dec 1, 2017

Choose a reason for hiding this comment

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

Then how can we guarantee this line won't throw exception?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Dec 1, 2017

Choose a reason for hiding this comment

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

We handled them in a missing columns in line 55.

}
}
}.toArray

var i = 0
while (i < length) {
val writable = fieldRefs(i)
if (writable == null) {
mutableRow.setNullAt(i)
} else {
Expand Down