You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2021/11/19 01:38:05 UTC

[Impala-ASF-CR] IMPALA-11028: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18037

Change subject: IMPALA-11028: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................

IMPALA-11028: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

In DescriptorTable.copyTupleDescriptor(), a TupleDescriptor object is
created with slots copied from source TupleDescriptor, but its "path"
member variable is set as null. This make the table associated with the
copied TupleDescriptor object detected as non Kudu table, and
SlotDescriptor.isKuduStringSlot() always returns false for its copied
slots.
TupleDescriptor.getSlotSize() add 4 padding bytes for Kudu string slot.
When this function is called to calculate slot size for string type
slots for copied TupleDescriptor object, the slot size will be 4 bytes
less than the slot size calculated for the source TupleDescriptor if the
table associated with source TupleDescriptor is Kudu table. This cause
Preconditions.checkState(d.getByteSize() == src.getByteSize()) failed to
throw exception.
To fix it, we should copy "path" from source TupleDescriptor for the
copied TupleDescriptor object before coping slots.

Testing:
- Added a new test case to verify the bug fixing.
- Passed exhaustive tests.

Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
---
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M tests/query_test/test_kudu.py
2 files changed, 51 insertions(+), 1 deletion(-)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/18037/1
--
To view, visit http://gerrit.cloudera.org:8080/18037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18037 )

Change subject: IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7668/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 23:30:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18037 )

Change subject: IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................

IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

In DescriptorTable.copyTupleDescriptor(), a TupleDescriptor object is
created with slots copied from source TupleDescriptor, but its "path"
member variable is set as null. This make the table associated with the
copied TupleDescriptor object detected as non Kudu table, and
SlotDescriptor.isKuduStringSlot() always returns false for its copied
slots.
TupleDescriptor.getSlotSize() add 4 padding bytes for Kudu string slot.
When this function is called to calculate slot size for string type
slots for copied TupleDescriptor object, the slot size will be 4 bytes
less than the slot size calculated for the source TupleDescriptor if the
table associated with source TupleDescriptor is Kudu table. This cause
Preconditions.checkState(d.getByteSize() == src.getByteSize()) failed to
throw exception.
To fix it, we should copy "path" from source TupleDescriptor for the
copied TupleDescriptor object before copying slots.

Testing:
 - Added a new test case to verify the bug fixing.
 - Passed exhaustive tests.

Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Reviewed-on: http://gerrit.cloudera.org:8080/18037
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M tests/query_test/test_kudu.py
2 files changed, 51 insertions(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.

Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18037 )

Change subject: IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18037/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18037/2//COMMIT_MSG@23
PS2, Line 23: coping
> nit: copying
fixed


http://gerrit.cloudera.org:8080/#/c/18037/2/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:

http://gerrit.cloudera.org:8080/#/c/18037/2/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java@81
PS2, Line 81:     if (src.getPath() != null) d.setPath(src.getPath());
> This change makes sense to me. I was thinking why we only see the issue for
You are right.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 21:22:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.

Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18037 )

Change subject: IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................


Patch Set 5:

Hit IMPALA-9057, which is irrelevant to this code change


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 23:43:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18037 )

Change subject: IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9810/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 05:02:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18037 )

Change subject: IMPALA-11029: DescriptorTable.copyTupleDescriptor throw exception for Kudu table
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88e005910134f2b7575c06fa02ce11890418a41
Gerrit-Change-Number: 18037
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Nov 2021 06:03:49 +0000
Gerrit-HasComments: No