[SPARK-42541][CONNECT] Support Pivot with provided pivot column values #40145
Conversation
There was a problem hiding this comment.
If v is a Column (which can happen because of the guard in the previous case statement) then this will produce an empty literal. The same applies to a scala.Symbol.
There was a problem hiding this comment.
Nice catch. v here actually should only refer to non-column case.
I think I was not certain how to handle when it is a column but not contains a literal. Any idea how to handle that case?
There was a problem hiding this comment.
I would throw an exception.
There was a problem hiding this comment.
done with a test.
There was a problem hiding this comment.
Please add tests for all errors thrown.
There was a problem hiding this comment.
So do you think if throw UnsupportedOperationException is ok for now? I cannot find a better way to re-use existing SQL errors at this moment?
There was a problem hiding this comment.
Basically what is the proper exception we should use for this case?
There was a problem hiding this comment.
Well we messed up. I guess IllegalArgumentException?
There was a problem hiding this comment.
Fixed. Because this is internal API I switched to use assert because there shouldn't a empty pivot proto which is a bug that we need to fix.
There was a problem hiding this comment.
oops yes. Updated
|
Merging |
### What changes were proposed in this pull request? Support Pivot with provided pivot column values. Not supporting Pivot without providing column values because that requires to do max value check which depends on the implementation of Spark configuration in Spark Connect. ### Why are the changes needed? API coverage ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? UT Closes #40145 from amaliujia/rw-pivot. Authored-by: Rui Wang <rui.wang@databricks.com> Signed-off-by: Herman van Hovell <herman@databricks.com> (cherry picked from commit 34a2d95) Signed-off-by: Herman van Hovell <herman@databricks.com>
### What changes were proposed in this pull request? Support Pivot with provided pivot column values. Not supporting Pivot without providing column values because that requires to do max value check which depends on the implementation of Spark configuration in Spark Connect. ### Why are the changes needed? API coverage ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? UT Closes apache#40145 from amaliujia/rw-pivot. Authored-by: Rui Wang <rui.wang@databricks.com> Signed-off-by: Herman van Hovell <herman@databricks.com> (cherry picked from commit 34a2d95) Signed-off-by: Herman van Hovell <herman@databricks.com>
2 participants
What changes were proposed in this pull request?
Support Pivot with provided pivot column values. Not supporting Pivot without providing column values because that requires to do max value check which depends on the implementation of Spark configuration in Spark Connect.
Why are the changes needed?
API coverage
Does this PR introduce any user-facing change?
NO
How was this patch tested?
UT