You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/07/11 00:45:11 UTC

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog 

Hello Tianyi Wang, Vuk Ercegovac,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/10914

to review the following change.


Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................

IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

This adds support for INSERT and LOAD DATA statements when LocalCatalog
is enabled by fixing the following items:

* Remove some downcasts to HdfsTable in various Stmt classes and replace
  them with casts to FeFsTable.
* Stub out the "write access" checks to fake always being writable. Left
  a TODO to properly implement these.
* Implemented various 'getPartition()' calls which take a user-provided
  partition specification, used by INSERT and LOAD statements that
  specify an explicit target partition.
* Fixed the "prototype partition" created by LocalFsTable to not include
  a location field. This fixed insertion into dynamic partitions.

Additionally fixed a couple other issues which were exercised by the
e2e test coverage for load/insert:

* The LocalCatalog getDb() and getTable() calls were previously assuming
  that all callers passed pre-canonicalized table names, but that wasn't
  the case. This adds the necessary toLower() calls so that statements
  referencing capitalized table names work.

With this patch, most of the associated e2e tests pass, with the
exception of those that check permissions levels of the target
directories. Those calls are still stubbed out.

Overall, running 'run-tests.py -k "not hbase and not avro"' results in
about a 90% pass rate after this patch:

=====================================================
186 failed, 1657 passed, 56 skipped, 33 xfailed, 1 xpassed in 512.75 seconds
=====================================================

Remaining test failures seem mostly unrelated to the above changes and
will be addressed in continuing patches.

Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
19 files changed, 138 insertions(+), 68 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java@113
PS2, Line 113: HdfsTable.getPartition(table_, partitionSpec_)
> lost track of why we're going with the static accessor vs the instance-leve
because the implementation of this method only uses public methods that are also part of the interface. In Java 8 we could use "default methods" in the interface, but otherwise we'd have to copy-paste the code into both implementations.

Even though Impala 3.x can depend on Java 8, I think we want to preserve the ability to backport this to 2.x branch in case some users are interested in the feature, so I was trying to avoid Java 8-isms.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Jul 2018 18:26:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 6: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Jul 2018 22:55:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Jul 2018 16:20:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Tianyi Wang, Vuk Ercegovac, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10914

to look at the new patch set (#3).

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................

IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

This adds support for INSERT and LOAD DATA statements when LocalCatalog
is enabled by fixing the following items:

* Remove some downcasts to HdfsTable in various Stmt classes and replace
  them with casts to FeFsTable.
* Stub out the "write access" checks to fake always being writable. Left
  a TODO to properly implement these.
* Implemented various 'getPartition()' calls which take a user-provided
  partition specification, used by INSERT and LOAD statements that
  specify an explicit target partition.
* Fixed the "prototype partition" created by LocalFsTable to not include
  a location field. This fixed insertion into dynamic partitions.

Additionally fixed a couple other issues which were exercised by the
e2e test coverage for load/insert:

* The LocalCatalog getDb() and getTable() calls were previously assuming
  that all callers passed pre-canonicalized table names, but that wasn't
  the case. This adds the necessary toLower() calls so that statements
  referencing capitalized table names work.

With this patch, most of the associated e2e tests pass, with the
exception of those that check permissions levels of the target
directories. Those calls are still stubbed out.

Overall, running 'run-tests.py -k "not hbase and not avro"' results in
about a 90% pass rate after this patch:

=====================================================
186 failed, 1657 passed, 56 skipped, 33 xfailed, 1 xpassed in 512.75 seconds
=====================================================

Remaining test failures seem mostly unrelated to the above changes and
will be addressed in continuing patches.

Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
19 files changed, 149 insertions(+), 69 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/10914/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10914
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2858/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Jul 2018 19:59:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java@113
PS2, Line 113: HdfsTable.getPartition(table_, partitionSpec_)
> because the implementation of this method only uses public methods that are
thx, makes sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Jul 2018 18:28:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java@113
PS2, Line 113: HdfsTable.getPartition(table_, partitionSpec_)
lost track of why we're going with the static accessor vs the instance-level accessor?


http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java:

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@111
PS2, Line 111: first
how is first defined (e.g., what determines the ordering of locations)?

what's returned if all locations have write access?


http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java:

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@157
PS2, Line 157: String partitionNotFoundMsg =
             :     
put this in a function so that this cost is paid only when there's an error.


http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@160
PS2, Line 160: an Hdfs
nit: an fs



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Jul 2018 17:39:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java:

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@111
PS2, Line 111: first
> how is first defined (e.g., what determines the ordering of locations)?
Done


http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java:

http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@157
PS2, Line 157: String partitionNotFoundMsg =
             :     
> put this in a function so that this cost is paid only when there's an error
Done


http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@160
PS2, Line 160: an Hdfs
> nit: an fs
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Jul 2018 01:10:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 2: Code-Review+2

remaining comments are primarily nits/clarifications for comments.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Jul 2018 00:05:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................

IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

This adds support for INSERT and LOAD DATA statements when LocalCatalog
is enabled by fixing the following items:

* Remove some downcasts to HdfsTable in various Stmt classes and replace
  them with casts to FeFsTable.
* Stub out the "write access" checks to fake always being writable. Left
  a TODO to properly implement these.
* Implemented various 'getPartition()' calls which take a user-provided
  partition specification, used by INSERT and LOAD statements that
  specify an explicit target partition.
* Fixed the "prototype partition" created by LocalFsTable to not include
  a location field. This fixed insertion into dynamic partitions.

Additionally fixed a couple other issues which were exercised by the
e2e test coverage for load/insert:

* The LocalCatalog getDb() and getTable() calls were previously assuming
  that all callers passed pre-canonicalized table names, but that wasn't
  the case. This adds the necessary toLower() calls so that statements
  referencing capitalized table names work.

With this patch, most of the associated e2e tests pass, with the
exception of those that check permissions levels of the target
directories. Those calls are still stubbed out.

Overall, running 'run-tests.py -k "not hbase and not avro"' results in
about a 90% pass rate after this patch:

=====================================================
186 failed, 1657 passed, 56 skipped, 33 xfailed, 1 xpassed in 512.75 seconds
=====================================================

Remaining test failures seem mostly unrelated to the above changes and
will be addressed in continuing patches.

Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Reviewed-on: http://gerrit.cloudera.org:8080/10914
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
19 files changed, 148 insertions(+), 71 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 8
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 05:19:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Vuk Ercegovac, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10914

to look at the new patch set (#6).

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................

IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

This adds support for INSERT and LOAD DATA statements when LocalCatalog
is enabled by fixing the following items:

* Remove some downcasts to HdfsTable in various Stmt classes and replace
  them with casts to FeFsTable.
* Stub out the "write access" checks to fake always being writable. Left
  a TODO to properly implement these.
* Implemented various 'getPartition()' calls which take a user-provided
  partition specification, used by INSERT and LOAD statements that
  specify an explicit target partition.
* Fixed the "prototype partition" created by LocalFsTable to not include
  a location field. This fixed insertion into dynamic partitions.

Additionally fixed a couple other issues which were exercised by the
e2e test coverage for load/insert:

* The LocalCatalog getDb() and getTable() calls were previously assuming
  that all callers passed pre-canonicalized table names, but that wasn't
  the case. This adds the necessary toLower() calls so that statements
  referencing capitalized table names work.

With this patch, most of the associated e2e tests pass, with the
exception of those that check permissions levels of the target
directories. Those calls are still stubbed out.

Overall, running 'run-tests.py -k "not hbase and not avro"' results in
about a 90% pass rate after this patch:

=====================================================
186 failed, 1657 passed, 56 skipped, 33 xfailed, 1 xpassed in 512.75 seconds
=====================================================

Remaining test failures seem mostly unrelated to the above changes and
will be addressed in continuing patches.

Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
19 files changed, 148 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/10914/6
-- 
To view, visit http://gerrit.cloudera.org:8080/10914
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/31/ : 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/10914
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Jul 2018 19:47:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 7: Code-Review+2

Forwarding +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 19:27:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 6:

Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/31/ 

Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Jul 2018 18:46:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 )

Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2859/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3
Gerrit-Change-Number: 10914
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 02:06:58 +0000
Gerrit-HasComments: No