Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19336 )

Change subject: IMPALA-11779: Fix crash in TopNNode due to slots in null type
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19336/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19336/2//COMMIT_MSG@9
PS2, Line 9: transfer
> Nit: transfers
Done


http://gerrit.cloudera.org:8080/#/c/19336/2//COMMIT_MSG@11
PS2, Line 11: Expr#
> nit:Expr
Done


http://gerrit.cloudera.org:8080/#/c/19336/2//COMMIT_MSG@24
PS2, Line 24: TYP
> Nit: of
Done


http://gerrit.cloudera.org:8080/#/c/19336/2/be/src/exec/topn-node.cc
File be/src/exec/topn-node.cc:

http://gerrit.cloudera.org:8080/#/c/19336/2/be/src/exec/topn-node.cc@103
PS2, Line 103: (conjuncts_.size(),
> to keep it consistent in the future, should we add a comment in the FE as 
> well?

We already have comments in FE, e.g.
https://github.com/apache/impala/blob/4cbc295c48de1b369a78ec266137dfc3b363e7ec/fe/src/main/java/org/apache/impala/analysis/Expr.java#L846-L848

"git grep TYPE_NULL fe" shows me more comments like this.

> To keep things cleaner and reusable, maybe we could also put this code into a 
> member function of SlotRef/ScalarExpr?

Good point! I looked at whether we have similar patterns in BE and found this:
https://github.com/apache/impala/blob/4cbc295c48de1b369a78ec266137dfc3b363e7ec/be/src/exec/grouping-aggregator.cc#L106-L109
I realized that we just need a SlotRef with a non-null type. Refactored the 
codes to use a type-safe create method for SlotRef.



--
To view, visit http://gerrit.cloudera.org:8080/19336
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6aaf80c5129eaf788c70c8f041021eaf73087f94
Gerrit-Change-Number: 19336
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Dec 2022 12:18:45 +0000
Gerrit-HasComments: Yes

Reply via email to