[GitHub] spark pull request #18754: [SPARK-21552][SQL] Add DecimalType support to Arr...

2017-12-26 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/18754


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #18754: [SPARK-21552][SQL] Add DecimalType support to Arr...

2017-12-24 Thread ueshin
Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/18754#discussion_r158620106
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/arrow/ArrowWriter.scala 
---
@@ -214,6 +216,22 @@ private[arrow] class DoubleWriter(val valueVector: 
Float8Vector) extends ArrowFi
   }
 }
 
+private[arrow] class DecimalWriter(
+val valueVector: DecimalVector,
+precision: Int,
+scale: Int) extends ArrowFieldWriter {
+
+  override def setNull(): Unit = {
+valueVector.setNull(count)
+  }
+
+  override def setValue(input: SpecializedGetters, ordinal: Int): Unit = {
+val decimal = input.getDecimal(ordinal, precision, scale)
+decimal.changePrecision(precision, scale)
--- End diff --

Unfortunately, it depends on the implementation of `getDecimal` for now.
Btw, I guess we need to check the return value of `changePrecision()` and 
set `null` if the value is `false`, which means overflow.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #18754: [SPARK-21552][SQL] Add DecimalType support to Arr...

2017-12-22 Thread BryanCutler
Github user BryanCutler commented on a diff in the pull request:

https://github.com/apache/spark/pull/18754#discussion_r158566190
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/arrow/ArrowWriter.scala 
---
@@ -214,6 +216,22 @@ private[arrow] class DoubleWriter(val valueVector: 
Float8Vector) extends ArrowFi
   }
 }
 
+private[arrow] class DecimalWriter(
+val valueVector: DecimalVector,
+precision: Int,
+scale: Int) extends ArrowFieldWriter {
+
+  override def setNull(): Unit = {
+valueVector.setNull(count)
+  }
+
+  override def setValue(input: SpecializedGetters, ordinal: Int): Unit = {
+val decimal = input.getDecimal(ordinal, precision, scale)
+decimal.changePrecision(precision, scale)
--- End diff --

Is it necessary to call `changePrecision` even though `getDecimal` already 
takes the precision/scale as input - is it not guaranteed to return a decimal 
with that scale?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #18754: [SPARK-21552][SQL] Add DecimalType support to Arr...

2017-12-22 Thread BryanCutler
Github user BryanCutler commented on a diff in the pull request:

https://github.com/apache/spark/pull/18754#discussion_r158567491
  
--- Diff: python/pyspark/sql/types.py ---
@@ -1617,7 +1617,7 @@ def to_arrow_type(dt):
 elif type(dt) == DoubleType:
 arrow_type = pa.float64()
 elif type(dt) == DecimalType:
-arrow_type = pa.decimal(dt.precision, dt.scale)
+arrow_type = pa.decimal128(dt.precision, dt.scale)
--- End diff --

yes, that's the right way - it is now fixed at 128 bits internally.  I 
believe the Arrow Java limit is the same as Spark 38/38, not sure if pyarrow is 
the same but I think so.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org