Skip to content

DRILL-6130: Fix NPE during physical plan submission for various storage plugins#1108

Closed
arina-ielchiieva wants to merge 1 commit into
apache:masterfrom
arina-ielchiieva:DRILL-6130
Closed

DRILL-6130: Fix NPE during physical plan submission for various storage plugins#1108
arina-ielchiieva wants to merge 1 commit into
apache:masterfrom
arina-ielchiieva:DRILL-6130

Conversation

@arina-ielchiieva
Copy link
Copy Markdown
Member

  1. Fixed ser / de issues for Hive, Kafka, Hbase plugins.
  2. Added physical plan submission unit test for all storage plugins in contrib module.
  3. Refactoring.

@arina-ielchiieva
Copy link
Copy Markdown
Member Author

@vdiravka please review.

Copy link
Copy Markdown
Member

@vdiravka vdiravka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor comments.
Except them +1, LGTM

@Override
public int getOperatorType() {
return CoreOperatorType.HBASE_SUB_SCAN_VALUE;
return -1;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add KUDU_SUB_SCAN_VALUE to CoreOperatorType and use here?

@@ -175,9 +175,8 @@ public void testBasicQueryWithNonExistentTableName() throws Exception {

@Test
public void testPhysicalPlanExecutionBasedOnQuery() throws Exception {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testPhysicalPlanExecutionBasedOnQuery -> testPhysicalPlanSubmission

…ge plugins

1. Fixed ser / de issues for Hive, Kafka, Hbase plugins.
2. Added physical plan submission unit test for all storage plugins in contrib module.
3. Refactoring.
@arina-ielchiieva
Copy link
Copy Markdown
Member Author

@vdiravka thanks for the code review. Addressed two comments.

@asfgit asfgit closed this in 58e4cec Feb 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants