jatin510 commented on PR #12718:
URL: https://github.com/apache/datafusion/pull/12718#issuecomment-2400045849

   > @jatin510 Great job! This PR is almost there ✊.
   > 
   > The `Rank` user-defined window function implementation is now split across 
three modules. This has resulted in duplicated code in many places.
   > 
   > It would be nicer if we continue to maintain the same form factor as the 
original implementation.
   > 
   > You can reuse the `RankType` enum used in the built-in window function 
implementation in `WindowUDF`:
   > 
   > ```rust
   > pub enum RankType {
   >     Basic,
   >     Dense,
   >     Percent,
   > }
   > ```
   > 
   > And in `PartitionEvaluator::evaluate` you can specialize like this,
   > 
   > ```rust
   > /// Evaluates the window function inside the given range.
   >     fn evaluate(
   >         &mut self,
   >         values: &[ArrayRef],
   >         range: &Range<usize>,
   >     ) -> Result<ScalarValue> {
   > 
   >       /// no more code duplication here ...
   > 
   >        match self.rank_type {
   >             RankType::Basic => Ok(ScalarValue::UInt64(Some(
   >                 self.state.last_rank_boundary as u64 + 1,
   >             ))),
   >             RankType::Dense => 
Ok(ScalarValue::UInt64(Some(self.state.n_rank as u64))),
   >             RankType::Percent => {
   >                 exec_err!("Can not execute PERCENT_RANK in a streaming 
fashion")
   >             }
   >         }
   > }
   > ```
   > 
   > Would you like to unify this? If not, this can be completed in a follow-on 
PR.
   
   I will create a follow up PR for this change


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to