You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hvanhovell <gi...@git.apache.org> on 2018/04/05 15:58:06 UTC
[GitHub] spark pull request #20986: [SPARL-23864][SQL] Add unsafe object writing to U...
GitHub user hvanhovell opened a pull request:
https://github.com/apache/spark/pull/20986
[SPARL-23864][SQL] Add unsafe object writing to UnsafeWriter
## What changes were proposed in this pull request?
This PR moves writing of `UnsafeRow`, `UnsafeArrayData` & `UnsafeMapData` out of the `GenerateUnsafeProjection`/`InterpretedUnsafeProjection` classes into the `UnsafeWriter` interface. This cleans up the code a little bit, and it should also result in less byte code for the code generated path.
## How was this patch tested?
Existing tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hvanhovell/spark SPARK-23864
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20986.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 #20986
----
commit 970232a719d2293ec872ef64c4f6fea7d8c727aa
Author: Herman van Hovell <hv...@...>
Date: 2018-04-05T15:47:42Z
Add unsafe object writing to UnsafeWriter
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20986
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88950/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2012/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88948/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2039/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20986
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179522607
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriter.java ---
@@ -103,42 +106,27 @@ protected final void zeroOutPaddingBytes(int numBytes) {
public abstract void write(int ordinal, Decimal input, int precision, int scale);
public final void write(int ordinal, UTF8String input) {
- final int numBytes = input.numBytes();
- final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(numBytes);
-
- // grow the global buffer before writing data.
- grow(roundedSize);
-
- zeroOutPaddingBytes(numBytes);
-
- // Write the bytes to the variable length portion.
- input.writeToMemory(getBuffer(), cursor());
-
- setOffsetAndSize(ordinal, numBytes);
-
- // move the cursor forward.
- increaseCursor(roundedSize);
+ writeUnalignedBytes(ordinal, input.getBaseObject(), input.getBaseOffset(), input.numBytes());
}
public final void write(int ordinal, byte[] input) {
write(ordinal, input, 0, input.length);
}
public final void write(int ordinal, byte[] input, int offset, int numBytes) {
- final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(input.length);
+ writeUnalignedBytes(ordinal, input, Platform.BYTE_ARRAY_OFFSET + offset, numBytes);
+ }
- // grow the global buffer before writing data.
+ private void writeUnalignedBytes(
+ int ordinal,
+ Object baseObject,
+ long baseOffset,
+ int numBytes) {
+ final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(numBytes);
--- End diff --
nit: 4 indents for arguments? At other places, too
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #88950 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88950/testReport)** for PR 20986 at commit [`8ef919f`](https://github.com/apache/spark/commit/8ef919f97e104f4d0ca7c6e9fa59e7d16a2974a2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #88982 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88982/testReport)** for PR 20986 at commit [`61ba88d`](https://github.com/apache/spark/commit/61ba88dfc2abe930b1924b836d1ba03c041cdd67).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` |class SpecificUnsafeProjection extends $`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179522301
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriter.java ---
@@ -156,6 +144,40 @@ public final void write(int ordinal, CalendarInterval input) {
increaseCursor(16);
}
+ public final void write(int ordinal, UnsafeRow row) {
+ writeAlignedBytes(ordinal, row.getBaseObject(), row.getBaseOffset(), row.getSizeInBytes());
+ }
+
+ public final void write(int ordinal, UnsafeMapData map) {
+ writeAlignedBytes(ordinal, map.getBaseObject(), map.getBaseOffset(), map.getSizeInBytes());
+ }
+
+ public final void write(UnsafeArrayData array) {
+ // Unsafe arrays both can be written as a regular array field or as part ofa map. This makes
+ // updating the offset and size dependent on the code path, this is why we currently do not
+ // provide an method for writing unsafe arrays that also updates the size and offset.
+ int numBytes = array.getSizeInBytes();
+ grow(numBytes);
+ Platform.copyMemory(
+ array.getBaseOffset(),
--- End diff --
`array.getBaseObject()`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2010/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #88982 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88982/testReport)** for PR 20986 at commit [`61ba88d`](https://github.com/apache/spark/commit/61ba88dfc2abe930b1924b836d1ba03c041cdd67).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179644816
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala ---
@@ -176,6 +141,23 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
""".trim
}
+
+ private def writeArrayToBuffer(
+ ctx: CodegenContext,
+ input: String,
+ index: String,
+ elementType: DataType,
+ rowWriter: String): String = {
+ val previousCursor = ctx.freshName("previousCursor")
+ s"""
+ // Remember the current cursor so that we can calculate how many bytes are
+ // written later.
+ final int $previousCursor = $rowWriter.cursor();
+ ${writeArrayToBuffer(ctx, input, elementType, rowWriter)}
+ $rowWriter.setOffsetAndSizeFromPreviousCursor($index, $previousCursor);
+ """
--- End diff --
+1
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179897021
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriter.java ---
@@ -103,42 +106,27 @@ protected final void zeroOutPaddingBytes(int numBytes) {
public abstract void write(int ordinal, Decimal input, int precision, int scale);
public final void write(int ordinal, UTF8String input) {
- final int numBytes = input.numBytes();
- final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(numBytes);
-
- // grow the global buffer before writing data.
- grow(roundedSize);
-
- zeroOutPaddingBytes(numBytes);
-
- // Write the bytes to the variable length portion.
- input.writeToMemory(getBuffer(), cursor());
-
- setOffsetAndSize(ordinal, numBytes);
-
- // move the cursor forward.
- increaseCursor(roundedSize);
+ writeUnalignedBytes(ordinal, input.getBaseObject(), input.getBaseOffset(), input.numBytes());
}
public final void write(int ordinal, byte[] input) {
write(ordinal, input, 0, input.length);
}
public final void write(int ordinal, byte[] input, int offset, int numBytes) {
- final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(input.length);
--- End diff --
Good catch!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179644530
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala ---
@@ -286,23 +239,32 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
Platform.putLong($rowWriter.getBuffer(), $tmpCursor - 8, $rowWriter.cursor() - $tmpCursor);
${writeArrayToBuffer(ctx, s"$tmpInput.valueArray()", valueType, rowWriter)}
+ $rowWriter.setOffsetAndSizeFromPreviousCursor($index, $previousCursor);
}
"""
}
- /**
- * If the input is already in unsafe format, we don't need to go through all elements/fields,
- * we can directly write it.
- */
- private def writeUnsafeData(ctx: CodegenContext, input: String, rowWriter: String) = {
- val sizeInBytes = ctx.freshName("sizeInBytes")
- s"""
- final int $sizeInBytes = $input.getSizeInBytes();
- // grow the global buffer before writing data.
- $rowWriter.grow($sizeInBytes);
- $input.writeToMemory($rowWriter.getBuffer(), $rowWriter.cursor());
- $rowWriter.increaseCursor($sizeInBytes);
- """
+ private def writeElement(
+ ctx: CodegenContext,
--- End diff --
4 space indentation
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by MarcSteven <gi...@git.apache.org>.
Github user MarcSteven commented on the issue:
https://github.com/apache/spark/pull/20986
cool
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #88950 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88950/testReport)** for PR 20986 at commit [`8ef919f`](https://github.com/apache/spark/commit/8ef919f97e104f4d0ca7c6e9fa59e7d16a2974a2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179644768
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala ---
@@ -176,6 +141,23 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
""".trim
}
+
+ private def writeArrayToBuffer(
+ ctx: CodegenContext,
--- End diff --
4 space indentation
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179522408
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriter.java ---
@@ -156,6 +144,40 @@ public final void write(int ordinal, CalendarInterval input) {
increaseCursor(16);
}
+ public final void write(int ordinal, UnsafeRow row) {
+ writeAlignedBytes(ordinal, row.getBaseObject(), row.getBaseOffset(), row.getSizeInBytes());
+ }
+
+ public final void write(int ordinal, UnsafeMapData map) {
+ writeAlignedBytes(ordinal, map.getBaseObject(), map.getBaseOffset(), map.getSizeInBytes());
+ }
+
+ public final void write(UnsafeArrayData array) {
+ // Unsafe arrays both can be written as a regular array field or as part ofa map. This makes
--- End diff --
nit: `ofa map` -> `of a map`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179867664
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriter.java ---
@@ -103,42 +106,27 @@ protected final void zeroOutPaddingBytes(int numBytes) {
public abstract void write(int ordinal, Decimal input, int precision, int scale);
public final void write(int ordinal, UTF8String input) {
- final int numBytes = input.numBytes();
- final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(numBytes);
-
- // grow the global buffer before writing data.
- grow(roundedSize);
-
- zeroOutPaddingBytes(numBytes);
-
- // Write the bytes to the variable length portion.
- input.writeToMemory(getBuffer(), cursor());
-
- setOffsetAndSize(ordinal, numBytes);
-
- // move the cursor forward.
- increaseCursor(roundedSize);
+ writeUnalignedBytes(ordinal, input.getBaseObject(), input.getBaseOffset(), input.numBytes());
}
public final void write(int ordinal, byte[] input) {
write(ordinal, input, 0, input.length);
}
public final void write(int ordinal, byte[] input, int offset, int numBytes) {
- final int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(input.length);
--- End diff --
I am accidentally fixing a bug here :)
cc @kiszk
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #88948 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88948/testReport)** for PR 20986 at commit [`970232a`](https://github.com/apache/spark/commit/970232a719d2293ec872ef64c4f6fea7d8c727aa).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #89112 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89112/testReport)** for PR 20986 at commit [`352c735`](https://github.com/apache/spark/commit/352c735ea54a17ef55a9740ad3ae9b163f982539).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20986
Merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20986
cc @kiszk @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20986
@kiszk @cloud-fan I made all code generation in `GenerateUnsafeProjection` follow the code template standard of using pipes as a margin and `stripmargin` at the end. The only downside is that this makes the diff a bit larger and git blame does not work as well.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2143/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #88948 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88948/testReport)** for PR 20986 at commit [`970232a`](https://github.com/apache/spark/commit/970232a719d2293ec872ef64c4f6fea7d8c727aa).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179525056
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala ---
@@ -176,6 +141,23 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
""".trim
}
+
+ private def writeArrayToBuffer(
+ ctx: CodegenContext,
+ input: String,
+ index: String,
+ elementType: DataType,
+ rowWriter: String): String = {
+ val previousCursor = ctx.freshName("previousCursor")
+ s"""
+ // Remember the current cursor so that we can calculate how many bytes are
+ // written later.
+ final int $previousCursor = $rowWriter.cursor();
+ ${writeArrayToBuffer(ctx, input, elementType, rowWriter)}
+ $rowWriter.setOffsetAndSizeFromPreviousCursor($index, $previousCursor);
+ """
--- End diff --
Is it better to use
```
s"""
|
""".stripMargin
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88982/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20986
**[Test build #89112 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89112/testReport)** for PR 20986 at commit [`352c735`](https://github.com/apache/spark/commit/352c735ea54a17ef55a9740ad3ae9b163f982539).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARL-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20986
nit: `[SPARL-23864]` -> `[SPARK-23864]` in title :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20986: [SPARK-23864][SQL] Add unsafe object writing to U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20986#discussion_r179645207
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala ---
@@ -176,6 +141,23 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
""".trim
}
+
+ private def writeArrayToBuffer(
--- End diff --
It's weird to have 2 `writeArrayToBuffer`. Since this one is only called in one place, can we inline it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20986: [SPARK-23864][SQL] Add unsafe object writing to UnsafeWr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20986
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89112/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org