pranav.lo...@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/19474 )
Change subject: IMPALA-11803: Fix hitting DCHECK when running union on empty table with MT_DOP>1 ...................................................................... Patch Set 6: (9 comments) > Patch Set 5: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/19474/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19474/2//COMMIT_MSG@7 PS2, Line 7: Fix hitting DCHECK when running union on empty table with MT_DOP>1 : > No, we just need to change the commit title. The JIRA title is ok to just d Sure, done http://gerrit.cloudera.org:8080/#/c/19474/2//COMMIT_MSG@11 PS2, Line 11: a > Not done yet in other lines.. Done http://gerrit.cloudera.org:8080/#/c/19474/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19474/3//COMMIT_MSG@12 PS3, Line 12: c > Nit: unnecessary leading space here and in the following lines. Done http://gerrit.cloudera.org:8080/#/c/19474/3//COMMIT_MSG@12 PS3, Line 12: on before se > Nit: returned from the computeNodeResourceProfile() function Done http://gerrit.cloudera.org:8080/#/c/19474/3//COMMIT_MSG@16 PS3, Line 16: Testing: The fix is verified with an end-to-end test in test_mt_dop.py. > Could mention that the added test is in test_mt_dop.py. Done http://gerrit.cloudera.org:8080/#/c/19474/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19474/4//COMMIT_MSG@14 PS4, Line 14: : > We usually have a "Testing" section in the commit message, you could put th Alright, I'll take care of it from next time. Done. http://gerrit.cloudera.org:8080/#/c/19474/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19474/5//COMMIT_MSG@15 PS5, Line 15: > I usually leave an empty line before the 'Testing' section and start the te Done http://gerrit.cloudera.org:8080/#/c/19474/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/19474/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@2095 PS3, Line 2095: // Update 'useMtScanNode_' before any return cases. It's used in BE. > Could you add a comment like this? So we won't make the same mistake when a Sure, done. http://gerrit.cloudera.org:8080/#/c/19474/3/tests/custom_cluster/test_mt_dop.py File tests/custom_cluster/test_mt_dop.py: http://gerrit.cloudera.org:8080/#/c/19474/3/tests/custom_cluster/test_mt_dop.py@70 PS3, Line 70: """ Regression test for IMPALA-11803: When used in DEBUG build, > We could include the Jira number here (IMPALA-11803) as a reference. The co Done -- To view, visit http://gerrit.cloudera.org:8080/19474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbae5e1a78211327a214b2d936743bda767ae3c4 Gerrit-Change-Number: 19474 Gerrit-PatchSet: 6 Gerrit-Owner: Anonymous Coward <pranav.lo...@cloudera.com> Gerrit-Reviewer: Anonymous Coward <pranav.lo...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Mon, 27 Feb 2023 10:25:55 +0000 Gerrit-HasComments: Yes
