The other alternative would be to populate it with the partitioning columns, if any. Thoughts?
Dimitris On Mon, May 8, 2017 at 4:02 PM, Lars Volker <l...@cloudera.com> wrote: > > 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 >>>>>> >>>>> >>>>> >>>> >>> >> >
