Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/22864 )
Change subject: IMPALA-14041: Enable planner tests ...................................................................... Patch Set 4: (2 comments) Is this something that should run as part of precommit? If so, we can add the test command to bin/run-all-tests.sh under the FE_TEST section. http://gerrit.cloudera.org:8080/#/c/22864/4/java/calcite-planner/src/test/java/org/apache/impala/TpcdsCpuCostPlannerTest.java File java/calcite-planner/src/test/java/org/apache/impala/TpcdsCpuCostPlannerTest.java: http://gerrit.cloudera.org:8080/#/c/22864/4/java/calcite-planner/src/test/java/org/apache/impala/TpcdsCpuCostPlannerTest.java@18 PS4, Line 18: package org.apache.impala.planner; Two things here: 1. We should have the package and directory structure line up. If something is in org.apache.impala.planner, it belongs in org/apache/impala/planner. IDEs complain about this. 2. I think it makes sense for the test code to be in org.apache.impala.calcite, since this maven project has all its code in that package. We don't want two different org.apache.impala.planner.TpcdsCpuCostPlannerTest classes to exist in Impala. http://gerrit.cloudera.org:8080/#/c/22864/4/java/calcite-planner/src/test/java/org/apache/impala/TpcdsCpuCostPlannerTest.java@105 PS4, Line 105: //setupAdmissionControl(); One of the differences from the regular TpcdsCpuCostPlannerTest is that we don't setup admission control. Is it that we don't need it because we aren't testing memory limits? -- To view, visit http://gerrit.cloudera.org:8080/22864 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idaab4e9068bb64e9a9ee12d83cd2b6b55b99b9bf Gerrit-Change-Number: 22864 Gerrit-PatchSet: 4 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Tue, 20 May 2025 22:27:17 +0000 Gerrit-HasComments: Yes
