impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Date Wed, 15 Feb 2017 05:41:41 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5840/2/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

Line 149: 
> Good point, though I'll just leave this as a TODO for now because we don't 
I think Thomas is right and the query may crash if there is a difference in column nullability.

We do rely on the nullability. The SlotDescriptors are set to nullable or non-nullable which
affects the memory layout of the tuple. If the tuples returned by Kudu are not 100% identical
to what Impala expects, you may get non-deterministic wrong results (and maybe a crash). See
the class comment in TupleDescriptor.java regarding the Kudu layout.

In response to one of your comments:
"After discussing with Dan on the Kudu team, we can make this simpler by checking at plan
time because the kudu scan token encodes the col metadata and deserializing it will fail if
the projection schema is no longer valid. The issue for us was that we did not check the Kudu
col type matched our col type at plan time."

Will the Kudu-side validation check that the projection is 100% identical to what the scan
token expects? Nullability etc. included?


http://gerrit.cloudera.org:8080/#/c/5840/6/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

Line 161:       if (!colType.matchesType(kuduColType)) {
Shouldn't this use equals() instead of matchesType()?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message