[GLUTEN-6803]Init ExpressionUtil.extendedExpressionTransformer in executor side #6837
Conversation
|
Run Gluten Clickhouse CI |
| @@ -354,6 +354,5 @@ object ExpressionMappings { | |||
| .toMap[Class[_], String] | |||
| } | |||
|
|
|||
| var expressionExtensionTransformer: ExpressionExtensionTrait = | |||
| DefaultExpressionExtensionTransformer() | |||
| var expressionExtensionTransformer: ExpressionExtensionTrait = _ | |||
There was a problem hiding this comment.
I'd suggest to use getter and setters to avoid directly accessing the variable.
In the setter, we can do a check to make sure it was not set.
Similarly in the getter, we can do a check to make sure it was set.
There was a problem hiding this comment.
@zhztheplayer Added the get and set method.
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
| private var expressionExtensionTransformer: Option[ExpressionExtensionTrait] = None | ||
|
|
||
| def getExpressionExtensionTransformer: ExpressionExtensionTrait = { | ||
| expressionExtensionTransformer.getOrElse(new DefaultExpressionExtensionTransformer) |
There was a problem hiding this comment.
Can we throw when it's none? Otherwise we don't make it sure whether the change (setting it in executor side) takes effect. Say if we throw when none, then we must set it in both driver and executor sides otherwise the application will fail by error.
There was a problem hiding this comment.
@zhztheplayer Yes. Updated.
|
Run Gluten Clickhouse CI |
zhouyuan
changed the title
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR was auto-closed because it has been stalled for 10 days with no activity. Please feel free to reopen if it is still valid. Thanks. |
What changes were proposed in this pull request?
Follow up #6803.
Init ExpressionUtil.extendedExpressionTransformer in GlutenExecutorPlugin side
How was this patch tested?
local verified.