You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgaido91 <gi...@git.apache.org> on 2018/10/01 16:32:08 UTC
[GitHub] spark pull request #22602: [SPARK-25582][SQL] Zero-out all bytes when writin...
GitHub user mgaido91 opened a pull request:
https://github.com/apache/spark/pull/22602
[SPARK-25582][SQL] Zero-out all bytes when writing decimal
## What changes were proposed in this pull request?
In #20850 when writing non-null decimals, instead of zero-ing all the 16 allocated bytes, we zero-out only the padding bytes. Since we always allocate 16 bytes, if the number of bytes needed for a decimal is lower than 9, then this means that the bytes between 8 and 16 are not zero-ed.
I see 2 solutions here:
- we can zero-out all the bytes in advance as it was done before #20850 (safer solution IMHO);
- we can allocate only the needed bytes (may be a bit more efficient in terms of memory used, but I have not investigated the feasibility of this option).
Hence I propose here the first solution in order to fix the correctness issue. We can eventually switch to the second if we think is more efficient later.
## How was this patch tested?
Running the test attached in the JIRA. I have not yet been able to write a good UT in order to reproduce the issue. If anyone has any suggestion it is more than welcomed. I'll try to find one in the next days anyway.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mgaido91/spark SPARK-25582
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22602.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 #22602
----
commit 851d7235cb2d60af95da3cde6420fea6e63c52d3
Author: Marco Gaido <ma...@...>
Date: 2018-10-01T16:22:34Z
[SPARK-25582][SQL] Zero-out all bytes when writing decimal
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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-unified/3625/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22602
@mgaido91 Could you change the title to [WIP] before you add the test case?
Also cc @hvanhovell @kiszk who are the best person to review these code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/22602
two minor comments. LGTM and good catch!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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-unified/3654/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96859/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22602
good catch! LGTM, waiting for the UT.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22602
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
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/22602#discussion_r221955172
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
+ decimal2.changePrecision(38, 18)
+ val unsafeRowWriter = new UnsafeRowWriter(1)
+ unsafeRowWriter.resetRowWriter()
+ unsafeRowWriter.write(0, decimal2, decimal2.precision, decimal2.scale)
+ unsafeRowWriter.reset()
+ unsafeRowWriter.write(0, decimal1, decimal1.precision, decimal1.scale)
+ val res = unsafeRowWriter.getRow
+ assert(res.getDecimal(0, decimal1.precision, decimal1.scale) == decimal1)
--- End diff --
then this doesn't demonstrate how this bug could affect correctness.
one better idea for this test: we create 2 row writers, one writes the decimal as what you did here, and the other one writes decimal `0.431` directly. Then we compare the 2 result rows and make sure they equals.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r221934230
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
+ decimal2.changePrecision(38, 18)
+ val unsafeRowWriter = new UnsafeRowWriter(1)
+ unsafeRowWriter.resetRowWriter()
+ unsafeRowWriter.write(0, decimal2, decimal2.precision, decimal2.scale)
+ unsafeRowWriter.reset()
+ unsafeRowWriter.write(0, decimal1, decimal1.precision, decimal1.scale)
+ val res = unsafeRowWriter.getRow
+ assert(res.getDecimal(0, decimal1.precision, decimal1.scale) == decimal1)
--- End diff --
this would pass also before the fix, as only the first 8 bytes are read. I added it as an additional safety fix.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22602
Thanks for the review @dongjoon-hyun
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96852/
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r221792922
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriter.java ---
@@ -185,13 +185,13 @@ public void write(int ordinal, Decimal input, int precision, int scale) {
// grow the global buffer before writing data.
holder.grow(16);
+ // zero-out the bytes
--- End diff --
nit: Can we refine a comment like the following to avoid this problem in the future?
```
// always zero-out the 16-byte buffer
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22602
I think we can create a `UnsafeWriterSuite` to do some low-level checking. We can leave the end-to-end test if it's too hard to write.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22602
thank you all for the reviews! I added the UT according to @cloud-fan's suggestion as I was unable to set up a reasonable the end-to-end UT. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22602
cc @cloud-fan @kiszk
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96859 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96859/testReport)** for PR 22602 at commit [`72b7c5c`](https://github.com/apache/spark/commit/72b7c5c16b33368bb332b168a99e99e13d4636cf).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96892 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96892/testReport)** for PR 22602 at commit [`d7d17d8`](https://github.com/apache/spark/commit/d7d17d8362912978851a55726ed2154435bebb2c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r222067876
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
--- End diff --
I don't think it is necessary, as we may want to include here also tests for other `UnsafeWriter` in the future.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r222032210
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
--- End diff --
`UnsafeWriterSuite` -> `UnsafeRowWriterSuite`? Also, renaming the file?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96893 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96893/testReport)** for PR 22602 at commit [`be38c4c`](https://github.com/apache/spark/commit/be38c4c47524605c4661333900d7b6f83ae2ce5a).
* 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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96822/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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-unified/3629/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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-unified/3607/
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r222071791
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
--- End diff --
I don't think so. We had better have both `UnsafeRowWriterSuite` and `UnsafeWriterSuite`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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-unified/3630/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96892/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/22602
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96822 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96822/testReport)** for PR 22602 at commit [`851d723`](https://github.com/apache/spark/commit/851d7235cb2d60af95da3cde6420fea6e63c52d3).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22602
LGTM, pending jenkins
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96852 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96852/testReport)** for PR 22602 at commit [`6b84b41`](https://github.com/apache/spark/commit/6b84b41915ae184912922bb820a147060ac13afc).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22602
Thank you, @mgaido91 and all.
According to the all review comments, I'll merge this to `master` and `branch-2.4`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96857 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96857/testReport)** for PR 22602 at commit [`64f4ed0`](https://github.com/apache/spark/commit/64f4ed0e286d1a01192400203e167d046cb800f5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r222245966
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriterSuite.scala ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeRowWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
--- End diff --
thanks, I added a check for that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96892 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96892/testReport)** for PR 22602 at commit [`d7d17d8`](https://github.com/apache/spark/commit/d7d17d8362912978851a55726ed2154435bebb2c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class UnsafeRowWriterSuite extends SparkFunSuite `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
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/22602#discussion_r221913801
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
+ decimal2.changePrecision(38, 18)
+ val unsafeRowWriter = new UnsafeRowWriter(1)
+ unsafeRowWriter.resetRowWriter()
+ unsafeRowWriter.write(0, decimal2, decimal2.precision, decimal2.scale)
+ unsafeRowWriter.reset()
+ unsafeRowWriter.write(0, decimal1, decimal1.precision, decimal1.scale)
+ val res = unsafeRowWriter.getRow
+ assert(res.getDecimal(0, decimal1.precision, decimal1.scale) == decimal1)
--- End diff --
Actually I think this assert is strong enough, we don't need to check the zero bytes below.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96857 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96857/testReport)** for PR 22602 at commit [`64f4ed0`](https://github.com/apache/spark/commit/64f4ed0e286d1a01192400203e167d046cb800f5).
* 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 pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r221960064
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
+ decimal2.changePrecision(38, 18)
+ val unsafeRowWriter = new UnsafeRowWriter(1)
+ unsafeRowWriter.resetRowWriter()
+ unsafeRowWriter.write(0, decimal2, decimal2.precision, decimal2.scale)
+ unsafeRowWriter.reset()
+ unsafeRowWriter.write(0, decimal1, decimal1.precision, decimal1.scale)
+ val res = unsafeRowWriter.getRow
+ assert(res.getDecimal(0, decimal1.precision, decimal1.scale) == decimal1)
--- End diff --
thanks for the suggestion @cloud-fan, I like this approach.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r222241581
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriterSuite.scala ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeRowWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
--- End diff --
Shall we verify the number of bytes?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96822 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96822/testReport)** for PR 22602 at commit [`851d723`](https://github.com/apache/spark/commit/851d7235cb2d60af95da3cde6420fea6e63c52d3).
* 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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/22602
Thank you. The first option looks good. Let me think about a good UT, too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96893/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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-unified/3655/
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
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/22602#discussion_r221913364
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeWriterSuite.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
+ decimal2.changePrecision(38, 18)
+ val unsafeRowWriter = new UnsafeRowWriter(1)
+ unsafeRowWriter.resetRowWriter()
+ unsafeRowWriter.write(0, decimal2, decimal2.precision, decimal2.scale)
+ unsafeRowWriter.reset()
+ unsafeRowWriter.write(0, decimal1, decimal1.precision, decimal1.scale)
+ val res = unsafeRowWriter.getRow
+ assert(res.getDecimal(0, decimal1.precision, decimal1.scale) == decimal1)
+ // Check that the bytes which are not used by decimal1 (but are allocated) are zero-ed out
+ assert(res.getBytes()(25) == 0x00)
--- End diff --
can we add a comment about how we get the `25`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96859 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96859/testReport)** for PR 22602 at commit [`72b7c5c`](https://github.com/apache/spark/commit/72b7c5c16b33368bb332b168a99e99e13d4636cf).
* 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 #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22602
Sorry, I realize only now I linked the wrong JIRA. This is for 25538. Unfortunately I am not in front of my laptop right now so I cannot update the title. I'll do asap. Sorry for the mistake. Thanks for understanding.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
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 #22602: [SPARK-25582][SQL] Zero-out all bytes when writing decim...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22602
cc @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96857/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96852 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96852/testReport)** for PR 22602 at commit [`6b84b41`](https://github.com/apache/spark/commit/6b84b41915ae184912922bb820a147060ac13afc).
* 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 pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
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/22602#discussion_r221809826
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriter.java ---
@@ -185,13 +185,13 @@ public void write(int ordinal, Decimal input, int precision, int scale) {
// grow the global buffer before writing data.
holder.grow(16);
+ // zero-out the bytes
--- 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 #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22602
**[Test build #96893 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96893/testReport)** for PR 22602 at commit [`be38c4c`](https://github.com/apache/spark/commit/be38c4c47524605c4661333900d7b6f83ae2ce5a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22602: [SPARK-25538][SQL] Zero-out all bytes when writin...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22602#discussion_r222241193
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriterSuite.scala ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.Decimal
+
+class UnsafeRowWriterSuite extends SparkFunSuite {
+
+ test("SPARK-25538: zero-out all bits for decimals") {
+ // This decimal holds 8 bytes
+ val decimal1 = Decimal(0.431)
+ decimal1.changePrecision(38, 18)
+ // This decimal holds 11 bytes
+ val decimal2 = Decimal(123456789.1232456789)
+ decimal2.changePrecision(38, 18)
+ // On an UnsafeRowWriter we write decimal2 first and then decimal1
+ val unsafeRowWriter1 = new UnsafeRowWriter(1)
+ unsafeRowWriter1.resetRowWriter()
+ unsafeRowWriter1.write(0, decimal2, decimal2.precision, decimal2.scale)
+ unsafeRowWriter1.reset()
+ unsafeRowWriter1.write(0, decimal1, decimal1.precision, decimal1.scale)
+ val res1 = unsafeRowWriter1.getRow
+ // On a second UnsafeRowWriter we write directly decimal2
--- End diff --
`... we write directly decimal1`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22602: [SPARK-25538][SQL] Zero-out all bytes when writing decim...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22602
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org