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