Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4622: Add ALTER COLUMN statement.
......................................................................


Patch Set 2:

> To be a bit more prescriptive than my last comment:
 > 
 > Can you see how much of this new functionality you can actually
 > accomplish using the change syntax given it's already there?
 > 
 > I think we'll have a harder time justifying this new syntax to
 > accomplish something you can technically already do, even if it is
 > superior. It'll require a bit more discussion.

Its certainly possible to do this just with the CHANGE syntax, you would just 
have to specify the column name twice and the type even if those aren't 
changing, as we discussed with Greg. Given that, I still think its the right 
choice to add the new syntax.

It doesn't work right now - for Kudu, CHANGE only supports renaming columns, so 
specifying a different type gives you an AnalysisException, specifying both 
column names as the same gets a 'column already exists' error back from Kudu, 
specifying a comment gets dropped silently, and specifying the Kudu-specific 
options gets an 'Unsupported option' analysis error, but all of those things 
are easily fixable.

At the very least, you're right that this can be implemented using the existing 
code for CHANGE, so I should be able to simplify what I've got here.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2e8bd65342b79644a0fdcd925e6f17797e89ad6
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: No

Reply via email to