[
https://issues.apache.org/jira/browse/IMPALA-11046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17455276#comment-17455276
]
Steve Carlin commented on IMPALA-11046:
---------------------------------------
Ah, I see.
I won't push hard on this, but one general comment as to why I'd still like to
make the change:
When we run it on jenkins, A DCHECK will bring down the server. Once the server
is brought down, it will fail many other tests behind it, thus potentially
masking some errors which would only be exposed on the next Jenkins run.
It's also a slight pain on the debugging side for me on my local machine since
I'd have to restart the server (but yeah, this is minor as well).
But I suppose these are not serious issues.
> When GetTupleIdx fails, it should return INVALID_IDX, not bring down impalad
> ----------------------------------------------------------------------------
>
> Key: IMPALA-11046
> URL: https://issues.apache.org/jira/browse/IMPALA-11046
> Project: IMPALA
> Issue Type: Improvement
> Components: Backend
> Reporter: Steve Carlin
> Assignee: Steve Carlin
> Priority: Major
>
> The following code exists in runtime/descriptors.cc:
> int RowDescriptor::GetTupleIdx(TupleId id) const {
> DCHECK_LT(id, tuple_idx_map_.size()) << "RowDescriptor: " << DebugString();
> return tuple_idx_map_[id];
> }
>
> If the id doesn't exist in the map, it returns INVALID_IDX. However, if the
> id >= tuple_idx_size, it crashes the server.
> I was working on an issue on the frontend where I passed an incorrect index
> and it failed the query when I passed a bad index in both instances, but it
> was much preferable not to crash the server and only fail the query. So the
> proposal here is to get rid of the DCHECK_LT and replace it with a "return
> INVALID_IDX" when it fails.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
