Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8572 )

Change subject: IMPALA-6184: Clean up after ScalarExprEvaluator::Clone() fails
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8572/1/be/src/exprs/scalar-expr-evaluator.cc
File be/src/exprs/scalar-expr-evaluator.cc:

http://gerrit.cloudera.org:8080/#/c/8572/1/be/src/exprs/scalar-expr-evaluator.cc@193
PS1, Line 193:     Status status =
> Kinda subtle, can you add a comment like the one above?
Done


http://gerrit.cloudera.org:8080/#/c/8572/1/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
File testdata/workloads/functional-query/queries/QueryTest/udf-errors.test:

http://gerrit.cloudera.org:8080/#/c/8572/1/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test@85
PS1, Line 85: on (bad_expr2(rand()) = (t2.bool_col && t1.bool_col));
> Is this one guaranteed to fail? It seems like if we're unlucky, rand() coul
PS2 has been updated to fail more deterministically. rand() with the same seed 
should generate the same sequence of numbers so in theory it should work but 
let's make things simpler. I added rand() to avoid the FE from mucking with the 
expression due to constant folding.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45ffd722d0a69ad05ae3c748cf504c7f1a959a1d
Gerrit-Change-Number: 8572
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Nov 2017 19:55:05 +0000
Gerrit-HasComments: Yes

Reply via email to