impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Harrison Sheinblatt (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4482: Use ALTER TABLE / RECOVER PARTITIONS when loading tpcds.store sales
Date Wed, 23 Nov 2016 22:57:40 GMT
Harrison Sheinblatt has posted comments on this change.

Change subject: IMPALA-4482: Use ALTER TABLE / RECOVER PARTITIONS when loading tpcds.store_sales
......................................................................


Patch Set 2:

I think this is an OK solution to unblock current work given we have Jiras for key improvements:
simplifying the calling method and considering using 'recover partitions' generally for the
'new' (or newly used) use case where we have loaded the HDFS snapshot but not the metadata
snapshot.

I do suggest the following:
1. Add a comment in this 'alter' section that the statement is for the code path where we
have restored the HDFS snapshot but still need to generate the metadata.
2. File a new Jira to investigate other cases where there may be a problem with this use case
or the .test sections aren't used as intended.  E.g. in alltypeserror in functional_schema_template.sql,
there is no ALTER section, but the CREATE section adds partitions explicitly.
3. Update the jira to refactor the calling python method to link the other jiras if not already,
and mention that we want all 3 key code paths to be simpler in comments.

I think it's too hard to make larger scale changes now given the priority of the dependent
work.  The reasons are:
1. We'd have to fix a lot of .test file sections to be consistent, this would require a lot
of time going through them all and a lot of testing given that there are at least 3 code paths
through each test file to verify.
2. Pulling the key logic up one level into the python requires a large refactor which should
probably be accomplished when we refactor the key method there (jira already exists).  Right
now that method is too complicated to make changes to and test quickly.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaae97d1d44201aeeacacdd39adbae35753512950
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Harrison Sheinblatt <hs7@hotmail.com>
Gerrit-HasComments: No

Mime
View raw message