Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/13680#discussion_r73949824 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/UnsafeArraySuite.scala --- @@ -18,27 +18,131 @@ package org.apache.spark.sql.catalyst.util import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder import org.apache.spark.sql.catalyst.expressions.UnsafeArrayData +import org.apache.spark.unsafe.Platform class UnsafeArraySuite extends SparkFunSuite { - test("from primitive int array") { - val array = Array(1, 10, 100) - val unsafe = UnsafeArrayData.fromPrimitiveArray(array) - assert(unsafe.numElements == 3) - assert(unsafe.getSizeInBytes == 4 + 4 * 3 + 4 * 3) - assert(unsafe.getInt(0) == 1) - assert(unsafe.getInt(1) == 10) - assert(unsafe.getInt(2) == 100) + val booleanArray = Array(false, true) + val shortArray = Array(1.toShort, 10.toShort, 100.toShort) + val intArray = Array(1, 10, 100) + val longArray = Array(1.toLong, 10.toLong, 100.toLong) + val floatArray = Array(1.1.toFloat, 2.2.toFloat, 3.3.toFloat) + val doubleArray = Array(1.1, 2.2, 3.3) + val stringArray = Array("1", "10", "100") --- End diff -- @davies could you favor me? To be honest, I am not familiar with implementation of `Decimal`. Could you please let me know how I am wrong? I wrote the following test suite and got the generated code for projection. In generated code, lines 90 and 91 always uses 38 and 18 for precision and scale while I pass different ```BigDecimal``` in the test suite. These code are generated at [here](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala#L218) and [here](https://github.com/apache/spark/blob/23c58653f900bfb71ef2b3186a95ad2562c33969/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala). I suspect that these places refers to [DecimalType.SYSTEM_DEFAULT](https://github.com/apache/spark/blob/d5911d1173fe0872f21cae6c47abf8ff479345a4/sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala#L110). `DecimalType.SYSTEM_DEFAULT` uses 38 and 18. Should we use always these values (38 and 18)? Would it be possible to let us know what are other test cases that use ```DecimalType``` in ```UnsafeArrayData```? ```java val decimalArray = Array(BigDecimal("123").setScale(1, BigDecimal.RoundingMode.FLOOR)) test("read array") { val unsafeDecimal = ExpressionEncoder[Array[BigDecimal]].resolveAndBind(). toRow(decimalArray).getArray(0) assert(unsafeDecimal.isInstanceOf[UnsafeArrayData]) assert(unsafeDecimal.numElements == decimalArray.length) decimalArray.zipWithIndex.map { case (e, i) => print(s"e: $e, ${e.precision}, ${e.scale}\n") assert(unsafeDecimal.getDecimal(i, e.precision, e.scale) == e) } } ``` ```java /* 031 */ public UnsafeRow apply(InternalRow i) { /* 032 */ holder.reset(); /* 033 */ /* 034 */ rowWriter.zeroOutNullBytes(); /* 035 */ /* 036 */ /* 037 */ boolean isNull1 = i.isNullAt(0); /* 038 */ scala.math.BigDecimal[] value1 = isNull1 ? null : ((scala.math.BigDecimal[])i.get(0, null)); /* 039 */ ArrayData value = null; /* 040 */ /* 041 */ if (!isNull1) { /* 042 */ /* 043 */ Decimal[] convertedArray = null; /* 044 */ int dataLength = value1.length; /* 045 */ convertedArray = new Decimal[dataLength]; /* 046 */ /* 047 */ int loopIndex = 0; /* 048 */ while (loopIndex < dataLength) { /* 049 */ MapObjects_loopValue0 = (scala.math.BigDecimal) (value1[loopIndex]); /* 050 */ MapObjects_loopIsNull1 = MapObjects_loopValue0 == null; /* 051 */ /* 052 */ /* 053 */ boolean isNull2 = MapObjects_loopIsNull1; /* 054 */ final Decimal value2 = isNull2 ? null : org.apache.spark.sql.types.Decimal.apply(MapObjects_loopValue0); /* 055 */ isNull2 = value2 == null; /* 056 */ if (isNull2) { /* 057 */ convertedArray[loopIndex] = null; /* 058 */ } else { /* 059 */ convertedArray[loopIndex] = value2; /* 060 */ } /* 061 */ /* 062 */ loopIndex += 1; /* 063 */ } /* 064 */ /* 065 */ value = new org.apache.spark.sql.catalyst.util.GenericArrayData(convertedArray); /* 066 */ } /* 067 */ if (isNull1) { /* 068 */ rowWriter.setNullAt(0); /* 069 */ } else { /* 070 */ // Remember the current cursor so that we can calculate how many bytes are /* 071 */ // written later. /* 072 */ final int tmpCursor = holder.cursor; /* 073 */ /* 074 */ if (value instanceof UnsafeArrayData) { /* 075 */ /* 076 */ final int sizeInBytes = ((UnsafeArrayData) value).getSizeInBytes(); /* 077 */ // grow the global buffer before writing data. /* 078 */ holder.grow(sizeInBytes); /* 079 */ ((UnsafeArrayData) value).writeToMemory(holder.buffer, holder.cursor); /* 080 */ holder.cursor += sizeInBytes; /* 081 */ /* 082 */ } else { /* 083 */ final int numElements = value.numElements(); /* 084 */ arrayWriter.initialize(holder, numElements, 4); /* 085 */ /* 086 */ for (int index = 0; index < numElements; index++) { /* 087 */ if (value.isNullAt(index)) { /* 088 */ arrayWriter.setNull(index); /* 089 */ } else { /* 090 */ final Decimal element = value.getDecimal(index, 38, 18); /* 091 */ arrayWriter.write(index, element, 38, 18); /* 092 */ } /* 093 */ } /* 094 */ } /* 095 */ /* 096 */ rowWriter.setOffsetAndSize(0, tmpCursor, holder.cursor - tmpCursor); /* 097 */ rowWriter.alignToWords(holder.cursor - tmpCursor); /* 098 */ } /* 099 */ result.setTotalSize(holder.totalSize()); /* 100 */ return result; /* 101 */ } ```
--- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org