impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zoltan Ivanfi (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3973: add position and occurrence to instr()
Date Mon, 12 Sep 2016 10:55:38 GMT
Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-3973: add position and occurrence to instr()

Patch Set 13:


We should also update the documentation for instr(). Where can I do that?

 > FYI we should try to get query generator support for this, I filed:
File be/src/exprs/

Line 2064:   TestValue("instr('', '')", TYPE_INT, 0);
> would probably be good to add this test case for backwards search.
File be/src/exprs/

Line 292:   if (occurrence.val <= 0 || start_position.val == 0) return IntVal(0);
> is this what other systems do? i.e. do they silently return 0 or do they ra
Thanks for spotting this. An invalid value for start position is handled by silently returning
a 0, therefore I assumed that an invalid occurrence value is handled in the same way. But
in reality, the latter gives an error:

[Oracle][ODBC][Ora]ORA-01428: argument '0' is out of range (SQL-HY000)

PS13, Line 302: DCHECK

PS13, Line 317: DCHECK
> DCHECK_GT. but what is the meaning of this DCHECK?  search_start_pos is the

PS13, Line 319: std::min
> This is pretty confusing. I think this would be clearer if we did the min()

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9648de458d243306fa14adc5e7f7002bf6f67fd
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Zoltan Ivanfi <>
Gerrit-HasComments: Yes

View raw message