impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Knupp (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4482: Use ALTER TABLE / RECOVER PARTITIONS when loading sales
Date Fri, 27 Jan 2017 21:26:57 GMT
David Knupp has posted comments on this change.

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

Patch Set 7:

I'm not disputing you from a generic, syntactical standpoint -- but this is what I can tell.
The ALTER TABLE statement shows up exactly 100 times in the templates in testdata/datasets/.

This pattern is used 98 times:

  $ git grep 'ALTER TABLE {table_name}' testdata/datasets/ | wc -l

This pattern is used only twice:

  $ git grep 'ALTER TABLE {db_name}{db_suffix}.{table_name}' testdata/datasets/ | wc -l

And since those two instances are both conditional...

  1850:ALTER TABLE {db_name}{db_suffix}.{table_name} ADD IF NOT EXISTS PARTITION (year=2015,
  1851:ALTER TABLE {db_name}{db_suffix}.{table_name} ADD IF NOT EXISTS PARTITION (year=2010,
month=3); *guess* is that they don't ever get executed.

Also, if you look at the traceback I posted in an earlier comment, the error has nothing to
do with the syntax of the ALTER TABLE statement itself -- rather, it's something to do with
expansion of {db_name} in the schema template file, hence the KeyError on the string 'db_name'.

  Traceback (most recent call last):
    File "./testdata/bin/", line 753, in <module>
      test_vectors, sections, include_constraints, exclude_constraints, only_constraints)
    File "./testdata/bin/", line 658, in generate_statements
      output.create.append(use_db + alter.format(table_name=table_name))
  KeyError: 'db_name'

{db_name} expansion seems to work with other kind of statements produced by
-- just not ALTER TABLE statements. I don't know why. But there's an open bug to refactor, which is generally recognized to be a rat's nest of suspect

Since the overwhelming precedent in our code base (98:2) is to use 'ALTER TABLE {table_name}'
can I just do that here? I'll change the comment to read:

  -- {db_name} expansion does not seem to work with ALTER TABLE statements. See IMPALA-4005.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaae97d1d44201aeeacacdd39adbae35753512950
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <>
Gerrit-Reviewer: David Knupp <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Harrison Sheinblatt <>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <>
Gerrit-HasComments: No

View raw message