2010YOUY01 commented on PR #19587:
URL: https://github.com/apache/datafusion/pull/19587#issuecomment-3707577664
@Brijesh-Thakkar This PR seems need more effort to work through the
propagated test failures. The CI can get auto-triggered after you have some PR
merged, so if you prefer, you can open one to fix any typo, we can merge them
right away.
For the propagated testing failures, It's possible we have deal issues deep
down the existing implementations
I'm wondering we can hack through it like
```
// existing one
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.partial_cmp_impl(other, false)
}
// the behavior following standard SQL Nulls
fn partial_cmp_sql_nulls(&self, other: &Self) -> Option<Ordering> {
self.partial_cmp_impl(other, false)
}
fn partial_cmp_impl(&self, other: &self, null_comparable: bool) {
....
}
```
So this SQL compatible behavior can be helpful to a feature I'm working on:
https://github.com/apache/datafusion/pull/19609
Besides, if the existing implementation is using this less common null
comparison semantics, it's possible to cause bugs (because the comment didn't
say why it allows non-standard null semantics, so it's likely to get used
incorrectly), we still want to check it further by enforcing the standard
behavior, and fixing all the tests.
--
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]
