Github user BryanCutler commented on the issue:
https://github.com/apache/spark/pull/21650
>ehh .. @BryanCutler, WDYT about just doing the previous one for now? The
approach you suggested sounds efficient of course but.. here's not a hot path
so I think the previous way is fine too .. since that's a bit cleaner (but a
bit less efficient), and partly the code freeze is close
I didn't make the suggestion for performance, it was because looking at the
previous code took me a while before I realized the intent was to find the
first evaluable udf then all others matching that eval type. I think the
previous code kind of masked that and made it more complicated to follow.
I wasn't really sure how the expression tree was evaluated, so my
suggestion didn't handle chained expressions. The problem was the eval type was
being set when checking the children nodes, instead it should only be set after
all children are determined to be the same type. I'll update the above code
again, which passes all tests, as far as I can tell. I still prefer this
approach, but I'm not a sql expert ;)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
