----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25320/#review53197 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java <https://reviews.apache.org/r/25320/#comment92674> small recommendation: every branch in this seems to have the same if/else wrt fetching stuff from tbl or part. can we pull out cols, serlib, sd once at the beginning and have the rest of this just use it? slightly cleaner and less error prone. ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java <https://reviews.apache.org/r/25320/#comment92696> what about other cases? location, serde, etc? ql/src/test/queries/clientpositive/alter_partition_change_col.q <https://reviews.apache.org/r/25320/#comment92694> it'd be good to test: - dynamic partition case (no value for one partition specs, multiple partition specs). does this work? If it does, what happens if there are some partitions that cannot be changed? (some partitions already have a column x others don't) - tables with multiple partitions - reordering columns - null/default partition - negative cases (name clash, column doesn't exist, etc) ql/src/test/queries/clientpositive/alter_partition_change_col.q <https://reviews.apache.org/r/25320/#comment92701> it'd be more interesting to see the full table output. where one part is 10,0 and one is 14,4 ql/src/test/queries/clientpositive/alter_partition_change_col.q <https://reviews.apache.org/r/25320/#comment92702> same as above ql/src/test/queries/clientpositive/alter_partition_change_col.q <https://reviews.apache.org/r/25320/#comment92703> before changing the table too, you should try just removing it on the partition, that should yield a null in the dec col. - Gunther Hagleitner On Sept. 3, 2014, 11:57 p.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25320/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2014, 11:57 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-7971 > https://issues.apache.org/jira/browse/HIVE-7971 > > > Repository: hive-git > > > Description > ------- > > Allow change/replace/add column to work on partitions > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java e42bbdd > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 05cde3e > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 25cd3a5 > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java 8517319 > ql/src/test/queries/clientpositive/alter_partition_change_col.q > PRE-CREATION > ql/src/test/results/clientpositive/alter_partition_change_col.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/25320/diff/ > > > Testing > ------- > > New qfile test added > > > Thanks, > > Jason Dere > >
