You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dafrista <gi...@git.apache.org> on 2016/06/15 00:43:39 UTC
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
GitHub user dafrista opened a pull request:
https://github.com/apache/spark/pull/13676
[SPARK-15956] [SQL] When unwrapping ORC avoid pattern matching at runtime
## What changes were proposed in this pull request?
Extend the returning of unwrapper functions from primitive types to all types.
## How was this patch tested?
The patch should pass all unit tests. Reading ORC files with non-primitive types with this change reduced the read time by ~15%.
===
The github diff is very noisy. Attaching the screenshots below for improved readability:
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dafrista/spark improve-orc-master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13676.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #13676
----
commit bdd188546c6043b694e2b1e023a4e78f1e5fffdb
Author: Brian Cho <bc...@fb.com>
Date: 2016-06-13T20:36:17Z
Avoid pattern matching in runtime when unwrapping ORC.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67277135
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,7 +354,287 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantDoubleObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBooleanObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantLongObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantFloatObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantShortObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantByteObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBinaryObjectInspector =>
+ data: Any => {
+ val writable = poi.getWritableConstantValue
+ val temp = new Array[Byte](writable.getLength)
+ System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
+ temp
+ }
+ case poi: WritableConstantDateObjectInspector =>
+ data: Any =>
+ DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
+ case mi: StandardConstantMapObjectInspector =>
+ val keyUnwrapper = unwrapperFor(mi.getMapKeyObjectInspector)
+ val valueUnwrapper = unwrapperFor(mi.getMapValueObjectInspector)
+ data: Any => {
+ // take the value from the map inspector object, rather than the input data
+ val keyValues = mi.getWritableConstantValue.asScala.toSeq
+ val keys = keyValues.map(kv => keyUnwrapper(kv._1)).toArray
+ val values = keyValues.map(kv => valueUnwrapper(kv._2)).toArray
+ ArrayBasedMapData(keys, values)
+ }
+ case li: StandardConstantListObjectInspector =>
+ val unwrapper = unwrapperFor(li.getListElementObjectInspector)
+ data: Any => {
+ // take the value from the list inspector object, rather than the input data
+ val values = li.getWritableConstantValue.asScala
+ .map(unwrapper)
+ .toArray
+ new GenericArrayData(values)
+ }
+ case poi: VoidObjectInspector =>
+ data: Any =>
+ null // always be null for void object inspector
+ case pi: PrimitiveObjectInspector => pi match {
+ // We think HiveVarchar/HiveChar is also a String
+ case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
+ data: Any => {
+ if (data != null) {
--- End diff --
There are many data: Any => { if (data != null) { ... ... }}. how about use one function to define it to decrease
codes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/13676
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67639405
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,8 +340,299 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Builds unwrappers ahead of time according to object inspector
* types to avoid pattern matching and branching costs per row.
+ *
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ *
+ * @param objectInspector the ObjectInspector used to create an unwrapper.
+ * @return A function that unwraps data objects.
+ * Use the overloaded HiveStructField version for in-place updating of a MutableRow.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantDoubleObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBooleanObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantLongObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantFloatObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantShortObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantByteObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBinaryObjectInspector =>
+ data: Any => {
+ val writable = poi.getWritableConstantValue
+ val temp = new Array[Byte](writable.getLength)
+ System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
+ temp
+ }
+ case poi: WritableConstantDateObjectInspector =>
+ data: Any =>
+ DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
+ case mi: StandardConstantMapObjectInspector =>
+ val keyUnwrapper = unwrapperFor(mi.getMapKeyObjectInspector)
+ val valueUnwrapper = unwrapperFor(mi.getMapValueObjectInspector)
+ data: Any => {
+ // take the value from the map inspector object, rather than the input data
+ val keyValues = mi.getWritableConstantValue.asScala.toSeq
+ val keys = keyValues.map(kv => keyUnwrapper(kv._1)).toArray
+ val values = keyValues.map(kv => valueUnwrapper(kv._2)).toArray
+ ArrayBasedMapData(keys, values)
+ }
+ case li: StandardConstantListObjectInspector =>
+ val unwrapper = unwrapperFor(li.getListElementObjectInspector)
+ data: Any => {
+ // take the value from the list inspector object, rather than the input data
+ val values = li.getWritableConstantValue.asScala
+ .map(unwrapper)
+ .toArray
+ new GenericArrayData(values)
+ }
+ case poi: VoidObjectInspector =>
+ data: Any =>
+ null // always be null for void object inspector
+ case pi: PrimitiveObjectInspector => pi match {
+ // We think HiveVarchar/HiveChar is also a String
+ case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
+ data: Any => {
+ if (data != null) {
+ UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveVarchar.getValue)
+ } else {
+ null
+ }
+ }
+ case hvoi: HiveVarcharObjectInspector =>
+ data: Any => {
+ if (data != null) {
+ UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
+ } else {
+ null
+ }
+ }
+ case hvoi: HiveCharObjectInspector if hvoi.preferWritable() =>
+ data: Any => {
+ if (data != null) {
+ UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveChar.getValue)
+ } else {
+ null
+ }
+ }
+ case hvoi: HiveCharObjectInspector =>
+ data: Any => {
+ if (data != null) {
+ UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
+ } else {
+ null
+ }
+ }
+ case x: StringObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) {
+ // Text is in UTF-8 already. No need to convert again via fromString. Copy bytes
+ val wObj = x.getPrimitiveWritableObject(data)
+ val result = wObj.copyBytes()
+ UTF8String.fromBytes(result, 0, result.length)
+ } else {
+ null
+ }
+ }
+ case x: StringObjectInspector =>
+ data: Any => {
+ if (data != null) {
+ UTF8String.fromString(x.getPrimitiveJavaObject(data))
+ } else {
+ null
+ }
+ }
+ case x: IntObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: BooleanObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: FloatObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: DoubleObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: LongObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: ShortObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: ByteObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) x.get(data) else null
+ }
+ case x: HiveDecimalObjectInspector =>
+ data: Any => {
+ if (data != null) {
+ HiveShim.toCatalystDecimal(x, data)
+ } else {
+ null
+ }
+ }
+ case x: BinaryObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) {
+ // BytesWritable.copyBytes() only available since Hadoop2
+ // In order to keep backward-compatible, we have to copy the
+ // bytes with old apis
+ val bw = x.getPrimitiveWritableObject(data)
+ val result = new Array[Byte](bw.getLength())
+ System.arraycopy(bw.getBytes(), 0, result, 0, bw.getLength())
+ result
+ } else {
+ null
+ }
+ }
+ case x: DateObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.fromJavaDate(x.getPrimitiveWritableObject(data).get())
+ } else {
+ null
+ }
+ }
+ case x: DateObjectInspector =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.fromJavaDate(x.getPrimitiveJavaObject(data))
+ } else {
+ null
+ }
+ }
+ case x: TimestampObjectInspector if x.preferWritable() =>
+ data: Any => {
+ if (data != null) {
+ val t = x.getPrimitiveWritableObject(data)
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ } else {
+ null
+ }
+ }
+ case ti: TimestampObjectInspector =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.fromJavaTimestamp(ti.getPrimitiveJavaObject(data))
+ } else {
+ null
+ }
+ }
+ case _ =>
+ data: Any => {
+ if (data != null) {
+ pi.getPrimitiveJavaObject(data)
+ } else {
+ null
+ }
+ }
+ }
+ case li: ListObjectInspector =>
+ val unwrapper = unwrapperFor(li.getListElementObjectInspector)
+ data: Any => {
+ if (data != null) {
+ Option(li.getList(data))
+ .map { l =>
+ val values = l.asScala.map(unwrapper).toArray
+ new GenericArrayData(values)
+ }
+ .orNull
+ } else {
+ null
+ }
+ }
+ case mi: MapObjectInspector =>
+ val keyUnwrapper = unwrapperFor(mi.getMapKeyObjectInspector)
+ val valueUnwrapper = unwrapperFor(mi.getMapValueObjectInspector)
+ data: Any => {
+ if (data != null) {
+ val map = mi.getMap(data)
+ if (map == null) {
+ null
+ } else {
+ val keyValues = map.asScala.toSeq
+ val keys = keyValues.map(kv => keyUnwrapper(kv._1)).toArray
+ val values = keyValues.map(kv => valueUnwrapper(kv._2)).toArray
+ ArrayBasedMapData(keys, values)
+ }
+ } else {
+ null
+ }
+ }
+ // currently, hive doesn't provide the ConstantStructObjectInspector
+ case si: StructObjectInspector =>
+ val fields = si.getAllStructFieldRefs.asScala
+ val fieldsToUnwrap = fields.zip(
+ fields.map(_.getFieldObjectInspector).map(unwrapperFor))
+ data: Any => {
+ if (data != null) {
+ InternalRow.fromSeq(fieldsToUnwrap.map(
--- End diff --
You can just can use a pattern match here to get to the field and the unwrap function directly, i.e.:
```scala
InternalRow.fromSeq(fieldsToUnwrap.map { case (field, unwrap) =>
unwrap(si.getStructFieldData(data, field))
}
```
That makes it a bit more readable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the issue:
https://github.com/apache/spark/pull/13676
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67281085
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,7 +354,287 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantDoubleObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBooleanObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantLongObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantFloatObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantShortObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantByteObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBinaryObjectInspector =>
+ data: Any => {
+ val writable = poi.getWritableConstantValue
+ val temp = new Array[Byte](writable.getLength)
+ System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
+ temp
+ }
+ case poi: WritableConstantDateObjectInspector =>
+ data: Any =>
+ DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
+ case mi: StandardConstantMapObjectInspector =>
+ val keyUnwrapper = unwrapperFor(mi.getMapKeyObjectInspector)
+ val valueUnwrapper = unwrapperFor(mi.getMapValueObjectInspector)
+ data: Any => {
+ // take the value from the map inspector object, rather than the input data
+ val keyValues = mi.getWritableConstantValue.asScala.toSeq
+ val keys = keyValues.map(kv => keyUnwrapper(kv._1)).toArray
+ val values = keyValues.map(kv => valueUnwrapper(kv._2)).toArray
+ ArrayBasedMapData(keys, values)
+ }
+ case li: StandardConstantListObjectInspector =>
+ val unwrapper = unwrapperFor(li.getListElementObjectInspector)
+ data: Any => {
+ // take the value from the list inspector object, rather than the input data
+ val values = li.getWritableConstantValue.asScala
+ .map(unwrapper)
+ .toArray
+ new GenericArrayData(values)
+ }
+ case poi: VoidObjectInspector =>
+ data: Any =>
+ null // always be null for void object inspector
+ case pi: PrimitiveObjectInspector => pi match {
+ // We think HiveVarchar/HiveChar is also a String
+ case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
+ data: Any => {
+ if (data != null) {
--- End diff --
i think the problem there is it might hurt perf.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13676
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/13676
This looks pretty good. What I am thinking is that generating an encoder could create another nice performance speedup here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by dafrista <gi...@git.apache.org>.
Github user dafrista commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67637381
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,8 +340,299 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Builds unwrappers ahead of time according to object inspector
* types to avoid pattern matching and branching costs per row.
+ *
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ *
+ * @param objectInspector the ObjectInspector used to create an unwrapper.
+ * @return A function that unwraps data objects.
+ * Use the overloaded HiveStructField version for in-place updating of a MutableRow.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
--- End diff --
You're right, as the contract for a `ConstantObjectInspector` is that its object "represent constant values and can return them without an evaluation" [[1](https://github.com/apache/hive/blob/master/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ConstantObjectInspector.java)]. I will make this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by dafrista <gi...@git.apache.org>.
Github user dafrista commented on the issue:
https://github.com/apache/spark/pull/13676
@rxin thanks for the review. I've added a commit that removes `unwrap` by replacing with the `unwrapperFor` pattern. (I've moved `unwrap` to tests, where it's much cleaner and no need for top performance.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67097521
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -243,137 +243,12 @@ private[hive] trait HiveInspectors {
* @param data the data in Hive type
* @param oi the ObjectInspector associated with the Hive Type
* @return convert the data into catalyst type
- * TODO return the function of (data => Any) instead for performance consideration
*
- * Strictly follows the following order in unwrapping (constant OI has the higher priority):
- * Constant Null object inspector =>
- * return null
- * Constant object inspector =>
- * extract the value from constant object inspector
- * Check whether the `data` is null =>
- * return null if true
- * If object inspector prefers writable =>
- * extract writable from `data` and then get the catalyst type from the writable
- * Extract the java object directly from the object inspector
- *
- * NOTICE: the complex data type requires recursive unwrapping.
+ * Use unwrapperFor's (data => Any) instead for performance consideration.
*/
- def unwrap(data: Any, oi: ObjectInspector): Any = oi match {
- case coi: ConstantObjectInspector if coi.getWritableConstantValue == null => null
- case poi: WritableConstantStringObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.toString)
- case poi: WritableConstantHiveVarcharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
- case poi: WritableConstantHiveCharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
- case poi: WritableConstantHiveDecimalObjectInspector =>
- HiveShim.toCatalystDecimal(
- PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
- poi.getWritableConstantValue.getHiveDecimal)
- case poi: WritableConstantTimestampObjectInspector =>
- val t = poi.getWritableConstantValue
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case poi: WritableConstantIntObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantDoubleObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBooleanObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantLongObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantFloatObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantShortObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantByteObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBinaryObjectInspector =>
- val writable = poi.getWritableConstantValue
- val temp = new Array[Byte](writable.getLength)
- System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
- temp
- case poi: WritableConstantDateObjectInspector =>
- DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
- case mi: StandardConstantMapObjectInspector =>
- // take the value from the map inspector object, rather than the input data
- val keyValues = mi.getWritableConstantValue.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- case li: StandardConstantListObjectInspector =>
- // take the value from the list inspector object, rather than the input data
- val values = li.getWritableConstantValue.asScala
- .map(unwrap(_, li.getListElementObjectInspector))
- .toArray
- new GenericArrayData(values)
- // if the value is null, we don't care about the object inspector type
- case _ if data == null => null
- case poi: VoidObjectInspector => null // always be null for void object inspector
- case pi: PrimitiveObjectInspector => pi match {
- // We think HiveVarchar/HiveChar is also a String
- case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveVarchar.getValue)
- case hvoi: HiveVarcharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case hvoi: HiveCharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveChar.getValue)
- case hvoi: HiveCharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case x: StringObjectInspector if x.preferWritable() =>
- // Text is in UTF-8 already. No need to convert again via fromString. Copy bytes
- val wObj = x.getPrimitiveWritableObject(data)
- val result = wObj.copyBytes()
- UTF8String.fromBytes(result, 0, result.length)
- case x: StringObjectInspector =>
- UTF8String.fromString(x.getPrimitiveJavaObject(data))
- case x: IntObjectInspector if x.preferWritable() => x.get(data)
- case x: BooleanObjectInspector if x.preferWritable() => x.get(data)
- case x: FloatObjectInspector if x.preferWritable() => x.get(data)
- case x: DoubleObjectInspector if x.preferWritable() => x.get(data)
- case x: LongObjectInspector if x.preferWritable() => x.get(data)
- case x: ShortObjectInspector if x.preferWritable() => x.get(data)
- case x: ByteObjectInspector if x.preferWritable() => x.get(data)
- case x: HiveDecimalObjectInspector => HiveShim.toCatalystDecimal(x, data)
- case x: BinaryObjectInspector if x.preferWritable() =>
- // BytesWritable.copyBytes() only available since Hadoop2
- // In order to keep backward-compatible, we have to copy the
- // bytes with old apis
- val bw = x.getPrimitiveWritableObject(data)
- val result = new Array[Byte](bw.getLength())
- System.arraycopy(bw.getBytes(), 0, result, 0, bw.getLength())
- result
- case x: DateObjectInspector if x.preferWritable() =>
- DateTimeUtils.fromJavaDate(x.getPrimitiveWritableObject(data).get())
- case x: DateObjectInspector => DateTimeUtils.fromJavaDate(x.getPrimitiveJavaObject(data))
- case x: TimestampObjectInspector if x.preferWritable() =>
- val t = x.getPrimitiveWritableObject(data)
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case ti: TimestampObjectInspector =>
- DateTimeUtils.fromJavaTimestamp(ti.getPrimitiveJavaObject(data))
- case _ => pi.getPrimitiveJavaObject(data)
- }
- case li: ListObjectInspector =>
- Option(li.getList(data))
- .map { l =>
- val values = l.asScala.map(unwrap(_, li.getListElementObjectInspector)).toArray
- new GenericArrayData(values)
- }
- .orNull
- case mi: MapObjectInspector =>
- val map = mi.getMap(data)
- if (map == null) {
- null
- } else {
- val keyValues = map.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- }
- // currently, hive doesn't provide the ConstantStructObjectInspector
- case si: StructObjectInspector =>
- val allRefs = si.getAllStructFieldRefs
- InternalRow.fromSeq(allRefs.asScala.map(
- r => unwrap(si.getStructFieldData(data, r), r.getFieldObjectInspector)))
+ def unwrap(data: Any, oi: ObjectInspector): Any = {
+ val unwrapper = unwrapperFor(oi)
+ unwrapper(data)
--- End diff --
can we work to remove the unwrap function here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by dafrista <gi...@git.apache.org>.
Github user dafrista commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67242472
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,7 +354,287 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantDoubleObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBooleanObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantLongObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantFloatObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantShortObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantByteObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBinaryObjectInspector =>
+ data: Any => {
+ val writable = poi.getWritableConstantValue
+ val temp = new Array[Byte](writable.getLength)
+ System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
+ temp
+ }
+ case poi: WritableConstantDateObjectInspector =>
+ data: Any =>
+ DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
+ case mi: StandardConstantMapObjectInspector =>
+ val keyUnwrapper = unwrapperFor(mi.getMapKeyObjectInspector)
+ val valueUnwrapper = unwrapperFor(mi.getMapValueObjectInspector)
+ data: Any => {
+ // take the value from the map inspector object, rather than the input data
+ val keyValues = mi.getWritableConstantValue.asScala.toSeq
+ val keys = keyValues.map(kv => keyUnwrapper(kv._1)).toArray
+ val values = keyValues.map(kv => valueUnwrapper(kv._2)).toArray
+ ArrayBasedMapData(keys, values)
+ }
+ case li: StandardConstantListObjectInspector =>
+ val unwrapper = unwrapperFor(li.getListElementObjectInspector)
+ data: Any => {
+ // take the value from the list inspector object, rather than the input data
+ val values = li.getWritableConstantValue.asScala
+ .map(unwrapper)
+ .toArray
+ new GenericArrayData(values)
+ }
+ case poi: VoidObjectInspector =>
+ data: Any =>
+ null // always be null for void object inspector
+ case pi: PrimitiveObjectInspector => pi match {
+ // We think HiveVarchar/HiveChar is also a String
+ case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
+ data: Any => {
+ if (data != null) {
--- End diff --
data is `Any` and not `AnyRef`, so I'm not sure using `ne` is straightforward.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67094026
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -243,137 +243,12 @@ private[hive] trait HiveInspectors {
* @param data the data in Hive type
* @param oi the ObjectInspector associated with the Hive Type
* @return convert the data into catalyst type
- * TODO return the function of (data => Any) instead for performance consideration
*
- * Strictly follows the following order in unwrapping (constant OI has the higher priority):
- * Constant Null object inspector =>
- * return null
- * Constant object inspector =>
- * extract the value from constant object inspector
- * Check whether the `data` is null =>
- * return null if true
- * If object inspector prefers writable =>
- * extract writable from `data` and then get the catalyst type from the writable
- * Extract the java object directly from the object inspector
- *
- * NOTICE: the complex data type requires recursive unwrapping.
+ * Use unwrapperFor's (data => Any) instead for performance consideration.
*/
- def unwrap(data: Any, oi: ObjectInspector): Any = oi match {
- case coi: ConstantObjectInspector if coi.getWritableConstantValue == null => null
- case poi: WritableConstantStringObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.toString)
- case poi: WritableConstantHiveVarcharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
- case poi: WritableConstantHiveCharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
- case poi: WritableConstantHiveDecimalObjectInspector =>
- HiveShim.toCatalystDecimal(
- PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
- poi.getWritableConstantValue.getHiveDecimal)
- case poi: WritableConstantTimestampObjectInspector =>
- val t = poi.getWritableConstantValue
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case poi: WritableConstantIntObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantDoubleObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBooleanObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantLongObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantFloatObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantShortObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantByteObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBinaryObjectInspector =>
- val writable = poi.getWritableConstantValue
- val temp = new Array[Byte](writable.getLength)
- System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
- temp
- case poi: WritableConstantDateObjectInspector =>
- DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
- case mi: StandardConstantMapObjectInspector =>
- // take the value from the map inspector object, rather than the input data
- val keyValues = mi.getWritableConstantValue.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- case li: StandardConstantListObjectInspector =>
- // take the value from the list inspector object, rather than the input data
- val values = li.getWritableConstantValue.asScala
- .map(unwrap(_, li.getListElementObjectInspector))
- .toArray
- new GenericArrayData(values)
- // if the value is null, we don't care about the object inspector type
- case _ if data == null => null
- case poi: VoidObjectInspector => null // always be null for void object inspector
- case pi: PrimitiveObjectInspector => pi match {
- // We think HiveVarchar/HiveChar is also a String
- case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveVarchar.getValue)
- case hvoi: HiveVarcharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case hvoi: HiveCharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveChar.getValue)
- case hvoi: HiveCharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case x: StringObjectInspector if x.preferWritable() =>
- // Text is in UTF-8 already. No need to convert again via fromString. Copy bytes
- val wObj = x.getPrimitiveWritableObject(data)
- val result = wObj.copyBytes()
- UTF8String.fromBytes(result, 0, result.length)
- case x: StringObjectInspector =>
- UTF8String.fromString(x.getPrimitiveJavaObject(data))
- case x: IntObjectInspector if x.preferWritable() => x.get(data)
- case x: BooleanObjectInspector if x.preferWritable() => x.get(data)
- case x: FloatObjectInspector if x.preferWritable() => x.get(data)
- case x: DoubleObjectInspector if x.preferWritable() => x.get(data)
- case x: LongObjectInspector if x.preferWritable() => x.get(data)
- case x: ShortObjectInspector if x.preferWritable() => x.get(data)
- case x: ByteObjectInspector if x.preferWritable() => x.get(data)
- case x: HiveDecimalObjectInspector => HiveShim.toCatalystDecimal(x, data)
- case x: BinaryObjectInspector if x.preferWritable() =>
- // BytesWritable.copyBytes() only available since Hadoop2
- // In order to keep backward-compatible, we have to copy the
- // bytes with old apis
- val bw = x.getPrimitiveWritableObject(data)
- val result = new Array[Byte](bw.getLength())
- System.arraycopy(bw.getBytes(), 0, result, 0, bw.getLength())
- result
- case x: DateObjectInspector if x.preferWritable() =>
- DateTimeUtils.fromJavaDate(x.getPrimitiveWritableObject(data).get())
- case x: DateObjectInspector => DateTimeUtils.fromJavaDate(x.getPrimitiveJavaObject(data))
- case x: TimestampObjectInspector if x.preferWritable() =>
- val t = x.getPrimitiveWritableObject(data)
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case ti: TimestampObjectInspector =>
- DateTimeUtils.fromJavaTimestamp(ti.getPrimitiveJavaObject(data))
- case _ => pi.getPrimitiveJavaObject(data)
- }
- case li: ListObjectInspector =>
- Option(li.getList(data))
- .map { l =>
- val values = l.asScala.map(unwrap(_, li.getListElementObjectInspector)).toArray
- new GenericArrayData(values)
- }
- .orNull
- case mi: MapObjectInspector =>
- val map = mi.getMap(data)
- if (map == null) {
- null
- } else {
- val keyValues = map.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- }
- // currently, hive doesn't provide the ConstantStructObjectInspector
- case si: StructObjectInspector =>
- val allRefs = si.getAllStructFieldRefs
- InternalRow.fromSeq(allRefs.asScala.map(
- r => unwrap(si.getStructFieldData(data, r), r.getFieldObjectInspector)))
+ def unwrap(data: Any, oi: ObjectInspector): Any = {
+ val unwrapper = unwrapperFor(oi)
+ unwrapper(data)
--- End diff --
I think this change can skip case match per row for complex data type in unwrap. because unwrap for complex data type return a function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/13676
I took a quick look and this looked reasonable. Would be great for somebody else to look at it more carefully too.
cc @hvanhovell
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/13676
Thanks :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67636421
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,8 +340,299 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Builds unwrappers ahead of time according to object inspector
* types to avoid pattern matching and branching costs per row.
+ *
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ *
+ * @param objectInspector the ObjectInspector used to create an unwrapper.
+ * @return A function that unwraps data objects.
+ * Use the overloaded HiveStructField version for in-place updating of a MutableRow.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
--- End diff --
Isn't it faster to call `poi.getWritableConstantValue.get()` outside of the function? And use the result in the function? Or am I missing something here? The same goes for all other constants.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by dafrista <gi...@git.apache.org>.
Github user dafrista commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67092384
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -243,137 +243,12 @@ private[hive] trait HiveInspectors {
* @param data the data in Hive type
* @param oi the ObjectInspector associated with the Hive Type
* @return convert the data into catalyst type
- * TODO return the function of (data => Any) instead for performance consideration
*
- * Strictly follows the following order in unwrapping (constant OI has the higher priority):
- * Constant Null object inspector =>
- * return null
- * Constant object inspector =>
- * extract the value from constant object inspector
- * Check whether the `data` is null =>
- * return null if true
- * If object inspector prefers writable =>
- * extract writable from `data` and then get the catalyst type from the writable
- * Extract the java object directly from the object inspector
- *
- * NOTICE: the complex data type requires recursive unwrapping.
+ * Use unwrapperFor's (data => Any) instead for performance consideration.
*/
- def unwrap(data: Any, oi: ObjectInspector): Any = oi match {
- case coi: ConstantObjectInspector if coi.getWritableConstantValue == null => null
- case poi: WritableConstantStringObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.toString)
- case poi: WritableConstantHiveVarcharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
- case poi: WritableConstantHiveCharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
- case poi: WritableConstantHiveDecimalObjectInspector =>
- HiveShim.toCatalystDecimal(
- PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
- poi.getWritableConstantValue.getHiveDecimal)
- case poi: WritableConstantTimestampObjectInspector =>
- val t = poi.getWritableConstantValue
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case poi: WritableConstantIntObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantDoubleObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBooleanObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantLongObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantFloatObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantShortObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantByteObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBinaryObjectInspector =>
- val writable = poi.getWritableConstantValue
- val temp = new Array[Byte](writable.getLength)
- System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
- temp
- case poi: WritableConstantDateObjectInspector =>
- DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
- case mi: StandardConstantMapObjectInspector =>
- // take the value from the map inspector object, rather than the input data
- val keyValues = mi.getWritableConstantValue.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- case li: StandardConstantListObjectInspector =>
- // take the value from the list inspector object, rather than the input data
- val values = li.getWritableConstantValue.asScala
- .map(unwrap(_, li.getListElementObjectInspector))
- .toArray
- new GenericArrayData(values)
- // if the value is null, we don't care about the object inspector type
- case _ if data == null => null
- case poi: VoidObjectInspector => null // always be null for void object inspector
- case pi: PrimitiveObjectInspector => pi match {
- // We think HiveVarchar/HiveChar is also a String
- case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveVarchar.getValue)
- case hvoi: HiveVarcharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case hvoi: HiveCharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveChar.getValue)
- case hvoi: HiveCharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case x: StringObjectInspector if x.preferWritable() =>
- // Text is in UTF-8 already. No need to convert again via fromString. Copy bytes
- val wObj = x.getPrimitiveWritableObject(data)
- val result = wObj.copyBytes()
- UTF8String.fromBytes(result, 0, result.length)
- case x: StringObjectInspector =>
- UTF8String.fromString(x.getPrimitiveJavaObject(data))
- case x: IntObjectInspector if x.preferWritable() => x.get(data)
- case x: BooleanObjectInspector if x.preferWritable() => x.get(data)
- case x: FloatObjectInspector if x.preferWritable() => x.get(data)
- case x: DoubleObjectInspector if x.preferWritable() => x.get(data)
- case x: LongObjectInspector if x.preferWritable() => x.get(data)
- case x: ShortObjectInspector if x.preferWritable() => x.get(data)
- case x: ByteObjectInspector if x.preferWritable() => x.get(data)
- case x: HiveDecimalObjectInspector => HiveShim.toCatalystDecimal(x, data)
- case x: BinaryObjectInspector if x.preferWritable() =>
- // BytesWritable.copyBytes() only available since Hadoop2
- // In order to keep backward-compatible, we have to copy the
- // bytes with old apis
- val bw = x.getPrimitiveWritableObject(data)
- val result = new Array[Byte](bw.getLength())
- System.arraycopy(bw.getBytes(), 0, result, 0, bw.getLength())
- result
- case x: DateObjectInspector if x.preferWritable() =>
- DateTimeUtils.fromJavaDate(x.getPrimitiveWritableObject(data).get())
- case x: DateObjectInspector => DateTimeUtils.fromJavaDate(x.getPrimitiveJavaObject(data))
- case x: TimestampObjectInspector if x.preferWritable() =>
- val t = x.getPrimitiveWritableObject(data)
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case ti: TimestampObjectInspector =>
- DateTimeUtils.fromJavaTimestamp(ti.getPrimitiveJavaObject(data))
- case _ => pi.getPrimitiveJavaObject(data)
- }
- case li: ListObjectInspector =>
- Option(li.getList(data))
- .map { l =>
- val values = l.asScala.map(unwrap(_, li.getListElementObjectInspector)).toArray
- new GenericArrayData(values)
- }
- .orNull
- case mi: MapObjectInspector =>
- val map = mi.getMap(data)
- if (map == null) {
- null
- } else {
- val keyValues = map.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- }
- // currently, hive doesn't provide the ConstantStructObjectInspector
- case si: StructObjectInspector =>
- val allRefs = si.getAllStructFieldRefs
- InternalRow.fromSeq(allRefs.asScala.map(
- r => unwrap(si.getStructFieldData(data, r), r.getFieldObjectInspector)))
+ def unwrap(data: Any, oi: ObjectInspector): Any = {
+ val unwrapper = unwrapperFor(oi)
+ unwrapper(data)
--- End diff --
There is no improvement when calling unwrap directly. This change moves the unwrap logic to unwrapperFor. This improves performance for ORC files, because `OrcFileFormat` instead calls unwrapperFor to return a function for each field, then unwraps each row with this function (see https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/orc/OrcFileFormat.scala#L356).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67091024
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -243,137 +243,12 @@ private[hive] trait HiveInspectors {
* @param data the data in Hive type
* @param oi the ObjectInspector associated with the Hive Type
* @return convert the data into catalyst type
- * TODO return the function of (data => Any) instead for performance consideration
*
- * Strictly follows the following order in unwrapping (constant OI has the higher priority):
- * Constant Null object inspector =>
- * return null
- * Constant object inspector =>
- * extract the value from constant object inspector
- * Check whether the `data` is null =>
- * return null if true
- * If object inspector prefers writable =>
- * extract writable from `data` and then get the catalyst type from the writable
- * Extract the java object directly from the object inspector
- *
- * NOTICE: the complex data type requires recursive unwrapping.
+ * Use unwrapperFor's (data => Any) instead for performance consideration.
*/
- def unwrap(data: Any, oi: ObjectInspector): Any = oi match {
- case coi: ConstantObjectInspector if coi.getWritableConstantValue == null => null
- case poi: WritableConstantStringObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.toString)
- case poi: WritableConstantHiveVarcharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
- case poi: WritableConstantHiveCharObjectInspector =>
- UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
- case poi: WritableConstantHiveDecimalObjectInspector =>
- HiveShim.toCatalystDecimal(
- PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
- poi.getWritableConstantValue.getHiveDecimal)
- case poi: WritableConstantTimestampObjectInspector =>
- val t = poi.getWritableConstantValue
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case poi: WritableConstantIntObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantDoubleObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBooleanObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantLongObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantFloatObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantShortObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantByteObjectInspector =>
- poi.getWritableConstantValue.get()
- case poi: WritableConstantBinaryObjectInspector =>
- val writable = poi.getWritableConstantValue
- val temp = new Array[Byte](writable.getLength)
- System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
- temp
- case poi: WritableConstantDateObjectInspector =>
- DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
- case mi: StandardConstantMapObjectInspector =>
- // take the value from the map inspector object, rather than the input data
- val keyValues = mi.getWritableConstantValue.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- case li: StandardConstantListObjectInspector =>
- // take the value from the list inspector object, rather than the input data
- val values = li.getWritableConstantValue.asScala
- .map(unwrap(_, li.getListElementObjectInspector))
- .toArray
- new GenericArrayData(values)
- // if the value is null, we don't care about the object inspector type
- case _ if data == null => null
- case poi: VoidObjectInspector => null // always be null for void object inspector
- case pi: PrimitiveObjectInspector => pi match {
- // We think HiveVarchar/HiveChar is also a String
- case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveVarchar.getValue)
- case hvoi: HiveVarcharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case hvoi: HiveCharObjectInspector if hvoi.preferWritable() =>
- UTF8String.fromString(hvoi.getPrimitiveWritableObject(data).getHiveChar.getValue)
- case hvoi: HiveCharObjectInspector =>
- UTF8String.fromString(hvoi.getPrimitiveJavaObject(data).getValue)
- case x: StringObjectInspector if x.preferWritable() =>
- // Text is in UTF-8 already. No need to convert again via fromString. Copy bytes
- val wObj = x.getPrimitiveWritableObject(data)
- val result = wObj.copyBytes()
- UTF8String.fromBytes(result, 0, result.length)
- case x: StringObjectInspector =>
- UTF8String.fromString(x.getPrimitiveJavaObject(data))
- case x: IntObjectInspector if x.preferWritable() => x.get(data)
- case x: BooleanObjectInspector if x.preferWritable() => x.get(data)
- case x: FloatObjectInspector if x.preferWritable() => x.get(data)
- case x: DoubleObjectInspector if x.preferWritable() => x.get(data)
- case x: LongObjectInspector if x.preferWritable() => x.get(data)
- case x: ShortObjectInspector if x.preferWritable() => x.get(data)
- case x: ByteObjectInspector if x.preferWritable() => x.get(data)
- case x: HiveDecimalObjectInspector => HiveShim.toCatalystDecimal(x, data)
- case x: BinaryObjectInspector if x.preferWritable() =>
- // BytesWritable.copyBytes() only available since Hadoop2
- // In order to keep backward-compatible, we have to copy the
- // bytes with old apis
- val bw = x.getPrimitiveWritableObject(data)
- val result = new Array[Byte](bw.getLength())
- System.arraycopy(bw.getBytes(), 0, result, 0, bw.getLength())
- result
- case x: DateObjectInspector if x.preferWritable() =>
- DateTimeUtils.fromJavaDate(x.getPrimitiveWritableObject(data).get())
- case x: DateObjectInspector => DateTimeUtils.fromJavaDate(x.getPrimitiveJavaObject(data))
- case x: TimestampObjectInspector if x.preferWritable() =>
- val t = x.getPrimitiveWritableObject(data)
- t.getSeconds * 1000000L + t.getNanos / 1000L
- case ti: TimestampObjectInspector =>
- DateTimeUtils.fromJavaTimestamp(ti.getPrimitiveJavaObject(data))
- case _ => pi.getPrimitiveJavaObject(data)
- }
- case li: ListObjectInspector =>
- Option(li.getList(data))
- .map { l =>
- val values = l.asScala.map(unwrap(_, li.getListElementObjectInspector)).toArray
- new GenericArrayData(values)
- }
- .orNull
- case mi: MapObjectInspector =>
- val map = mi.getMap(data)
- if (map == null) {
- null
- } else {
- val keyValues = map.asScala.toSeq
- val keys = keyValues.map(kv => unwrap(kv._1, mi.getMapKeyObjectInspector)).toArray
- val values = keyValues.map(kv => unwrap(kv._2, mi.getMapValueObjectInspector)).toArray
- ArrayBasedMapData(keys, values)
- }
- // currently, hive doesn't provide the ConstantStructObjectInspector
- case si: StructObjectInspector =>
- val allRefs = si.getAllStructFieldRefs
- InternalRow.fromSeq(allRefs.asScala.map(
- r => unwrap(si.getStructFieldData(data, r), r.getFieldObjectInspector)))
+ def unwrap(data: Any, oi: ObjectInspector): Any = {
+ val unwrapper = unwrapperFor(oi)
+ unwrapper(data)
--- End diff --
how would this actually improve performance if you are doing it per row?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pat...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13676#discussion_r67097541
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala ---
@@ -479,7 +354,287 @@ private[hive] trait HiveInspectors {
}
/**
- * Builds specific unwrappers ahead of time according to object inspector
+ * Strictly follows the following order in unwrapping (constant OI has the higher priority):
+ * Constant Null object inspector =>
+ * return null
+ * Constant object inspector =>
+ * extract the value from constant object inspector
+ * If object inspector prefers writable =>
+ * extract writable from `data` and then get the catalyst type from the writable
+ * Extract the java object directly from the object inspector
+ *
+ * NOTICE: the complex data type requires recursive unwrapping.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector): Any => Any =
+ objectInspector match {
+ case coi: ConstantObjectInspector if coi.getWritableConstantValue == null =>
+ data: Any => null
+ case poi: WritableConstantStringObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.toString)
+ case poi: WritableConstantHiveVarcharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveVarchar.getValue)
+ case poi: WritableConstantHiveCharObjectInspector =>
+ data: Any =>
+ UTF8String.fromString(poi.getWritableConstantValue.getHiveChar.getValue)
+ case poi: WritableConstantHiveDecimalObjectInspector =>
+ data: Any =>
+ HiveShim.toCatalystDecimal(
+ PrimitiveObjectInspectorFactory.javaHiveDecimalObjectInspector,
+ poi.getWritableConstantValue.getHiveDecimal)
+ case poi: WritableConstantTimestampObjectInspector =>
+ data: Any => {
+ val t = poi.getWritableConstantValue
+ t.getSeconds * 1000000L + t.getNanos / 1000L
+ }
+ case poi: WritableConstantIntObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantDoubleObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBooleanObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantLongObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantFloatObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantShortObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantByteObjectInspector =>
+ data: Any =>
+ poi.getWritableConstantValue.get()
+ case poi: WritableConstantBinaryObjectInspector =>
+ data: Any => {
+ val writable = poi.getWritableConstantValue
+ val temp = new Array[Byte](writable.getLength)
+ System.arraycopy(writable.getBytes, 0, temp, 0, temp.length)
+ temp
+ }
+ case poi: WritableConstantDateObjectInspector =>
+ data: Any =>
+ DateTimeUtils.fromJavaDate(poi.getWritableConstantValue.get())
+ case mi: StandardConstantMapObjectInspector =>
+ val keyUnwrapper = unwrapperFor(mi.getMapKeyObjectInspector)
+ val valueUnwrapper = unwrapperFor(mi.getMapValueObjectInspector)
+ data: Any => {
+ // take the value from the map inspector object, rather than the input data
+ val keyValues = mi.getWritableConstantValue.asScala.toSeq
+ val keys = keyValues.map(kv => keyUnwrapper(kv._1)).toArray
+ val values = keyValues.map(kv => valueUnwrapper(kv._2)).toArray
+ ArrayBasedMapData(keys, values)
+ }
+ case li: StandardConstantListObjectInspector =>
+ val unwrapper = unwrapperFor(li.getListElementObjectInspector)
+ data: Any => {
+ // take the value from the list inspector object, rather than the input data
+ val values = li.getWritableConstantValue.asScala
+ .map(unwrapper)
+ .toArray
+ new GenericArrayData(values)
+ }
+ case poi: VoidObjectInspector =>
+ data: Any =>
+ null // always be null for void object inspector
+ case pi: PrimitiveObjectInspector => pi match {
+ // We think HiveVarchar/HiveChar is also a String
+ case hvoi: HiveVarcharObjectInspector if hvoi.preferWritable() =>
+ data: Any => {
+ if (data != null) {
--- End diff --
you can get a tiny bit of per improvement by doing
```
if (data ne null) {
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/13676
LGTM - merging to master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13676: [SPARK-15956] [SQL] When unwrapping ORC avoid pattern ma...
Posted by dafrista <gi...@git.apache.org>.
Github user dafrista commented on the issue:
https://github.com/apache/spark/pull/13676
Thanks @hvanhovell I've pushed a commit addressing your comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org