Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17894 )

Change subject: IMPALA-10950: Update expr-benchmark.cc
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17894/3/be/src/benchmarks/expr-benchmark.cc
File be/src/benchmarks/expr-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/17894/3/be/src/benchmarks/expr-benchmark.cc@108
PS3, Line 108:     FragmentState* fragment_state = state->obj_pool()->Add(
> can you add context here about the kind of plan generated? maybe just the t
I'll add a query plan sample as a comment.


http://gerrit.cloudera.org:8080/#/c/17894/4/be/src/benchmarks/expr-benchmark.cc
File be/src/benchmarks/expr-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/17894/4/be/src/benchmarks/expr-benchmark.cc@78
PS4, Line 78: ity class
> nit: how about EnableCodegen(true)
Will do.


http://gerrit.cloudera.org:8080/#/c/17894/4/be/src/benchmarks/expr-benchmark.cc@629
PS4, Line 629: col",
> was the previous case not valid?
Right. This expression extract PROTOCOL, but the previous url does not have 
one, and the expression return an error.


http://gerrit.cloudera.org:8080/#/c/17894/5/be/src/benchmarks/expr-benchmark.cc
File be/src/benchmarks/expr-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/17894/5/be/src/benchmarks/expr-benchmark.cc@104
PS5, Line 104: new RuntimeState(query_ctx, &exec_env_);
> you can also allocate this from pool_ so that it can be cleaned eventually
Ack, I'll try it in the next patch set.


http://gerrit.cloudera.org:8080/#/c/17894/5/be/src/benchmarks/expr-benchmark.cc@171
PS5, Line 171: test_data
> does this ever get deleted?
Since I remove the test data vector and ReleaseTestData() method from Planner, 
this will stay until the benchmark program ends.
There is no crash though, because the destructor of ExprTestData will make sure 
to close all the objects when benchmark program ends.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b17434d85e32a58622bffb64a697b062a8bf43f
Gerrit-Change-Number: 17894
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Qifan Chen <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Thu, 07 Oct 2021 01:48:37 +0000
Gerrit-HasComments: Yes

Reply via email to