mikhailnik-db commented on code in PR #49907:
URL: https://github.com/apache/spark/pull/49907#discussion_r1955969177
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/SupportsOrderingWithinGroup.scala:
##########
@@ -26,20 +26,20 @@ trait SupportsOrderingWithinGroup { self: AggregateFunction
=>
def withOrderingWithinGroup(orderingWithinGroup: Seq[SortOrder]):
AggregateFunction
/** Indicator that ordering was set. */
- def orderingFilled: Boolean
+ def orderingFilled: Boolean = false
/**
* Tells Analyzer that WITHIN GROUP (ORDER BY ...) is mandatory for function.
*
* @see [[QueryCompilationErrors.functionMissingWithinGroupError]]
*/
- def isOrderingMandatory: Boolean
+ def isOrderingMandatory: Boolean = true
/**
* Tells Analyzer that DISTINCT is supported.
* The DISTINCT can conflict with order so some functions can ban it.
*
- * @see [[QueryCompilationErrors.functionMissingWithinGroupError]]
+ * @see
[[QueryCompilationErrors.distinctWithOrderingFunctionUnsupportedError]]
*/
- def isDistinctSupported: Boolean
+ def isDistinctSupported: Boolean = false
Review Comment:
I don't like the idea of default flags in this trait. I think every
developer should make a deliberate decision about what behavior he wants for a
new function. With default values, it's easy to forget some corner cases.
Personally, as a new contributor, I saw abstract/non-implemented methods and
flags as hints, which is not to forget. But about default values, you can get
to know them only by reading all the code or during testing.
@cloud-fan, what do you think?
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
