Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )
Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/13873/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13873/3//COMMIT_MSG@24 PS3, Line 24: * Since no new functionality has been added no tests were added > Can you add a test that exercises the option to confirm that it doesn't cra Done - added tests/query_test/test_result_spooling.py - doesn't do much yet, just validates that running a simple query with SPOOL_QUERY_RESULTS = true does not crash Impala. http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h File be/src/exec/blocking-plan-root-sink.h: http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h@18 PS3, Line 18: #ifndef IMPALA_EXEC_BLOCKING_PLAN_ROOT_SINK_H > #pragma once, since that's what we're doing in new code Done http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h@56 PS3, Line 56: virtual Status Send(RuntimeState* state, RowBatch* batch); > Can you add override annotations - that's a good practice for new code. Done http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h File be/src/exec/buffered-plan-root-sink.h: http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h@19 PS3, Line 19: #define IMPALA_EXEC_BUFFERED_PLAN_ROOT_SINK_H > #pragma once Done http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h@34 PS3, Line 34: virtual Status Send(RuntimeState* state, RowBatch* batch); > Can you add override annotations - that's a good practice for new code. Done http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/service/query-options.h@175 PS3, Line 175: ADVANCED > This should be a DEVELOPMENT query option at least until it's ready. Done -- To view, visit http://gerrit.cloudera.org:8080/13873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca Gerrit-Change-Number: 13873 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 17 Jul 2019 19:04:44 +0000 Gerrit-HasComments: Yes
