On Mon, May 8, 2017 at 11:41 PM, Marcel Kornacker <mar...@cloudera.com> wrote:
> > > On Mon, May 8, 2017 at 9:50 AM, Dimitris Tsirogiannis < > dtsirogian...@cloudera.com> wrote: > >> I believe it sends the wrong message and is probably confusing to throw >> an error when someone writes a CREATE TABLE with an empty SORT BY() but >> allow the same clause in an ALTER. No doubt the users can read the >> documentation and figure it out but its an extra step. Also, as Marcel >> mentions, scripting may be easier as you don't have to figure out whether >> to add a clause or not. Anyways, the patch is great as is, I just wanted to >> mention this. >> > > Sounds like we should allow an empty list then. > I will change the parser accordingly. On a related note, ALTER TABLE SORT BY () will leave an empty property 'sort.by.columns'. Should it remove the property altogether instead? > > >> >> >> Dimitris >> >> On Mon, May 8, 2017 at 7:50 AM, Marcel Kornacker <mar...@cloudera.com> >> wrote: >> >>> >>> >>> On Sat, May 6, 2017 at 7:57 PM, Dimitris Tsirogiannis < >>> dtsirogian...@cloudera.com> wrote: >>> >>>> For consistency, I believe we should at least allow an empty SORT BY() >>>> clause in the CREATE TABLE statement, but I'll defer the decision to Alex >>>> or Marcel. >>>> >>> >>> Do we think there'll be scripts generating create table statements with >>> empty sort-by clauses? I'm not sure there's a benefit to supporting that >>> (but happy to stand corrected). >>> >>> >>>> >>>> Dimitris >>>> >>>> On Sat, May 6, 2017 at 8:16 AM, Lars Volker (Code Review) < >>>> ger...@cloudera.org> wrote: >>>> >>>>> Lars Volker has posted comments on this change. >>>>> >>>>> Change subject: IMPALA-4166: Add SORT BY sql clause >>>>> ...................................................................... >>>>> >>>>> >>>>> Patch Set 20: >>>>> >>>>> > There is still one thing that is not clear to me. Why is it allowed >>>>> > to do an ALTER TABLE with an empty SORT BY clause but not a CREATE >>>>> > TABLE? >>>>> >>>>> The easiest way to specify an empty list of sort by columns during >>>>> CREATE TABLE is to omit the SORT BY clause altogether. To keep things >>>>> simple I did not add additional support for an empty SORT BY () clause. >>>>> >>>>> For ALTER TABLE there needs to be a way to specify an empty list of >>>>> columns, but since SORT BY is used to identify the command, the most >>>>> simple >>>>> form seemed to be an empty column list(). >>>>> >>>>> Do you think we should allow CREATE TABLE SORT BY() in addition to >>>>> specify an empty set of column? >>>>> >>>>> -- >>>>> To view, visit http://gerrit.cloudera.org:8080/6495 >>>>> To unsubscribe, visit http://gerrit.cloudera.org:8080/settings >>>>> >>>>> Gerrit-MessageType: comment >>>>> Gerrit-Change-Id: I08834f38a941786ab45a4381c2732d929a934f75 >>>>> Gerrit-PatchSet: 20 >>>>> Gerrit-Project: Impala-ASF >>>>> Gerrit-Branch: master >>>>> Gerrit-Owner: Lars Volker <l...@cloudera.com> >>>>> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> >>>>> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> >>>>> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> >>>>> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> >>>>> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> >>>>> Gerrit-HasComments: No >>>>> >>>> >>>> >>> >> >
