-
Notifications
You must be signed in to change notification settings - Fork 447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GLUTEN-6600]Fix NPE issue when running window sql #6803
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format?
See also: |
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
expressionExtensionTransformer.extensionExpressionsMapping | ||
|
||
var supportedExprs = defaultExpressionsMap | ||
if (expressionExtensionTransformer != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about giving expressionExtensionTransformer
a initial value DefaultExpressionExtensionTransformer
? it seems quite risk to check if not null at everywhere..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ulysses-you It's strange that the ExpressionUtil.extendedExpressionTransformer
method is returning null. I'm not sure why this is happening. It should be return the DefaultExpressionExtensionTransformer. Do you have any insights?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean ExpressionUtil.extendedExpressionTransformer
returns null but not DefaultExpressionExtensionTransformer()
when extendedExpressionTransformer
is empty ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zzcclp Yes.
Run Gluten Clickhouse CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you @JkSelf
What changes were proposed in this pull request?
When running the following sql in yarn client mode, we got the following exception:
How was this patch tested?
local verified.