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 <dkn...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Harrison Sheinblatt <h...@hotmail.com> Gerrit-HasComments: No
