You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/02 15:04:15 UTC

[GitHub] [spark] cloud-fan commented on pull request #27066: [SPARK-31317][SQL] Add withField method to Column

cloud-fan commented on pull request #27066:
URL: https://github.com/apache/spark/pull/27066#issuecomment-637601838


   Hi @fqaiser94 , I've sent you a PR to refactor the code a bit: https://github.com/fqaiser94/spark/pull/1
   1. remove the SQL API. It's unclear to me if we should add a special syntax for it, or simply treat it as a function. We can add the SQL API later.
   2. return null if the struct is null. I think the user-facing behavior matters more than the internal implementation. I don't think it's a big problem to have deeply nested if-else expressions. It's similar to the encoder expressions.
   3. remove `withFieldTestNamePrefix` in the test. It's pretty weird to see we construct test name like this, and it actually makes the test harder to read.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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