berkaysynnada commented on code in PR #21520:
URL: https://github.com/apache/datafusion/pull/21520#discussion_r3097264342
##########
datafusion/expr-common/src/interval_arithmetic.rs:
##########
@@ -929,7 +929,7 @@ impl Interval {
/// when the calculated cardinality does not fit in an `u64`.
pub fn cardinality(&self) -> Option<u64> {
let data_type = self.data_type();
- if data_type.is_integer() {
Review Comment:
Minor notice: perhaps it would be cleaner to gate on the concrete types
which `distance` actually handles (Date & Timestamp) now rather than the
broader` is_temporal()` check. Your call though. Once the remaining temporal
types (Time*, Duration*, Interval*) also get distance arms, this line won't
need to change. Just a bit confusing right now since the gate can be read as
"all temporal types supported" but in practice distance returns None for some
of them
--
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]
