Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4155: Update default partition when table is altered
......................................................................


Patch Set 2:

(8 comments)

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

Line 9: If the table format is changed by the Alter Table statement, the
> Also mention that the insert picks up the file format for new partitions fr
Done


Line 10: default partition in a partitioned tables does not get updated. This
> "in partitioned tables used to not get updated."
Done


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

Line 971:     // refer to this to understand how to create new partitions. If 
this method is called
> This new sentence should go into the method comment above. something like "
Removed the sentence and added a method comment.


http://gerrit.cloudera.org:8080/#/c/4750/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

Line 1872:       // The default partition must be updated if the file format is 
changed.
> brief explanation why, e.g., "so that new partitions are created with the n
Done


Line 1917:       if (tbl instanceof HdfsTable) ((HdfsTable) 
tbl).addDefaultPartition(msTbl.getSd());
> brief explanation why such as "Update default partition so that new partiti
It actually turns out that it's not necessary to update the location because 
that info is not stored in the default partition. Removed.


http://gerrit.cloudera.org:8080/#/c/4750/2/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:

Line 961: drop table if exists i3802_alter;
> No need for this drop if exists because we're using unique_database in the 
Done


Line 962: create table i3802_alter (c1 int)
> fix jira#
Done


Line 970: INT, STRING
> add another test for altering the location and check that the existing part
That functionality is removed, so the test is not needed any more.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59bf21caa5c5e7867d07d87cda0c0a5b4b994859
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to