impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Subject Re: [Impala-ASF-CR] IMPALA-4166: Add SORT BY sql clause
Date Mon, 08 May 2017 23:24:19 GMT
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 <lv@cloudera.com> wrote:

>
> On Mon, May 8, 2017 at 11:41 PM, Marcel Kornacker <marcel@cloudera.com>
> wrote:
>
>>
>>
>> On Mon, May 8, 2017 at 9:50 AM, Dimitris Tsirogiannis <
>> dtsirogiannis@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 <marcel@cloudera.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Sat, May 6, 2017 at 7:57 PM, Dimitris Tsirogiannis <
>>>> dtsirogiannis@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) <
>>>>> gerrit@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 <lv@cloudera.com>
>>>>>> Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
>>>>>> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
>>>>>> Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
>>>>>> Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
>>>>>> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
>>>>>> Gerrit-HasComments: No
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message