[SPARK-9368][SQL] Support get(ordinal, dataType) generic getter in UnsafeRow. #7682
Conversation
|
Test build #38497 has finished for PR 7682 at commit
|
Updated ExpressionEvalHelper to avoid conversion.
|
Test build #38498 has finished for PR 7682 at commit
|
| * The target memory address must already been allocated, and have enough space to hold all the | ||
| * bytes in this string. | ||
| */ | ||
| public void writeToMemory(Object target, long targetOffset) { |
There was a problem hiding this comment.
It looks like this method is unused? Are there other outstanding changes that you need to push?
There was a problem hiding this comment.
Not for this PR, but I'm using this in my struct type support PR.
|
Hmm, that |
|
LGTM. |
|
LGTM |
|
Jenkins, retest this please. |
|
Test build #38501 has finished for PR 7682 at commit
|
|
Thanks - I've merged this. |
| @@ -236,6 +236,41 @@ public Object get(int ordinal) { | |||
| } | |||
|
|
|||
| @Override | |||
| public Object get(int ordinal, DataType dataType) { | |||
| if (dataType instanceof NullType) { | |||
There was a problem hiding this comment.
I found a bug here: we should also check whether the row isNullAt(ordinal), in which case we should return null.
There was a problem hiding this comment.
This reminds me: there's a potentially subtle pitfall with UnsafeRow if someone calls a primitive type-specific accessor without first checking the nullability: in that case, we'll currently return 0 instead of throwing an error.
I'm going to add some assertions to try to see if there are any places where we make this mistake.
There was a problem hiding this comment.
Actually, it looks like our existing row behavior is to just return the zero-value of the given type for null inputs (e.g. getFloat on a null column returns 0.0f whereas the generic getter returns null). For some reason, it looks like UnsafeRow was returning NaN instead of 0 in those cases, leading to a confusing bug. I'm going to fix this inconsistency in a separate patch.
This pull request enables Unsafe mode by default in Spark SQL. In order to do this, we had to fix a number of small issues: **List of fixed blockers**: - [x] Make some default buffer sizes configurable so that HiveCompatibilitySuite can run properly (#7741). - [x] Memory leak on grouped aggregation of empty input (fixed by #7560 to fix this) - [x] Update planner to also check whether codegen is enabled before planning unsafe operators. - [x] Investigate failing HiveThriftBinaryServerSuite test. This turns out to be caused by a ClassCastException that occurs when Exchange tries to apply an interpreted RowOrdering to an UnsafeRow when range partitioning an RDD. This could be fixed by #7408, but a shorter-term fix is to just skip the Unsafe exchange path when RangePartitioner is used. - [x] Memory leak exceptions masking exceptions that actually caused tasks to fail (will be fixed by #7603). - [x] ~~https://issues.apache.org/jira/browse/SPARK-9162, to implement code generation for ScalaUDF. This is necessary for `UDFSuite` to pass. For now, I've just ignored this test in order to try to find other problems while we wait for a fix.~~ This is no longer necessary as of #7682. - [x] Memory leaks from Limit after UnsafeExternalSort cause the memory leak detector to fail tests. This is a huge problem in the HiveCompatibilitySuite (fixed by f4ac642a4e5b2a7931c5e04e086bb10e263b1db6). - [x] Tests in `AggregationQuerySuite` are failing due to NaN-handling issues in UnsafeRow, which were fixed in #7736. - [x] `org.apache.spark.sql.ColumnExpressionSuite.rand` needs to be updated so that the planner check also matches `TungstenProject`. - [x] After having lowered the buffer sizes to 4MB so that most of HiveCompatibilitySuite runs: - [x] Wrong answer in `join_1to1` (fixed by #7680) - [x] Wrong answer in `join_nulls` (fixed by #7680) - [x] Managed memory OOM / leak in `lateral_view` - [x] Seems to hang indefinitely in `partcols1`. This might be a deadlock in script transformation or a bug in error-handling code? The hang was fixed by #7710. - [x] Error while freeing memory in `partcols1`: will be fixed by #7734. - [x] After fixing the `partcols1` hang, it appears that a number of later tests have issues as well. - [x] Fix thread-safety bug in codegen fallback expression evaluation (#7759). Author: Josh Rosen <joshrosen@databricks.com> Closes #7564 from JoshRosen/unsafe-by-default and squashes the following commits: 83c0c56 [Josh Rosen] Merge remote-tracking branch 'origin/master' into unsafe-by-default f4cc859 [Josh Rosen] Merge remote-tracking branch 'origin/master' into unsafe-by-default 963f567 [Josh Rosen] Reduce buffer size for R tests d6986de [Josh Rosen] Lower page size in PySpark tests 013b9da [Josh Rosen] Also match TungstenProject in checkNumProjects 5d0b2d3 [Josh Rosen] Add task completion callback to avoid leak in limit after sort ea250da [Josh Rosen] Disable unsafe Exchange path when RangePartitioning is used 715517b [Josh Rosen] Enable Unsafe by default
No description provided.