flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] godfreyhe commented on issue #8051: [FLINK-12018] [table-planner-blink] Add support for generating optimized logical plan for Sort and Rank
Date Mon, 01 Apr 2019 03:06:30 GMT
godfreyhe commented on issue #8051:  [FLINK-12018] [table-planner-blink] Add support for generating
optimized logical plan for Sort and Rank
URL: https://github.com/apache/flink/pull/8051#issuecomment-478419996
 
 
   > > > > > Thanks for the refactoring and simplification. But it seems the
name "FirstLastRow" causes some misunderstandings. Not sure which of us has misunderstood
the purpose to introduce "FirstLastRow". From my understanding, this operator is more like
"RowDuplicate" operator (maybe we should rename it if i was right), it will duplicate rows
by some fields, and thus we have an option whether to keep the first row with the same key
or the last row in streaming case. I don't think the "order by limit x" clause should fall
into this kind of operator. Maybe i was wrong. What do you think?
   > > > > 
   > > > > 
   > > > > Yes, I also thing it more clear if we rename this operator to `RowDeduplicate`
and keep `first_row` and `last_row` as aggregate call
   > > > 
   > > > 
   > > > Isn't a boolean flag `keep_first` or `keep_last` enough?
   > > 
   > > 
   > > a boolean flag is enough, or a enum type: FIRST_ROW and LAST_ROW
   > 
   > I would prefer `keep_xxx`, it's more explicit to show what exactly we did.
   
   It makes sense to me.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message