Add isOnlyNotNull to Domain #17189
Add isOnlyNotNull to Domain #17189
Conversation
elonazoulay
force-pushed
the
domain_api
branch
2 times, most recently
from
921244c to
409c47d
Compare
| @@ -154,6 +154,11 @@ public boolean isOnlyNull() | |||
| return values.isNone() && nullAllowed; | |||
| } | |||
|
|
|||
| public boolean isOnlyNotNull() | |||
There was a problem hiding this comment.
Do we need this Only prefix here ? (No changes requested wanted to have a discussion here)
Cc: @findepi
There was a problem hiding this comment.
I saw the isOnlyNull method which (imo) makes it clear what the method means so I thought to keep the name consistent, i.e. if changed to isNotNull it could also imply always true, wdyt?
There was a problem hiding this comment.
isOnlyNull means "only null allowed" (null is just one possible value)
isOnlyNotNull means "only not null allowed", but there is no such single thing as "not null".
to avoid confusion, let's call the new method isAllExceptNull
There was a problem hiding this comment.
isOnlyNotNull means "only not null allowed", but there is no such single thing as "not null".
Not sure what you mean by this. "not null" means any value but null.
This method should be named isNotNull() (i.e., does this domain represent the set of values that are not null?), and it would be the counterpart of Domain.notNull(...). I don't see what's confusing about that.
There was a problem hiding this comment.
Thanks! Updated. Should the pr name be changed?
There was a problem hiding this comment.
To me, Domain is "a set", not "a filter".
isNotNull is suitable terminology for a filter, but not for the set
maybe isAllNonNull would be suitable for a set
elonazoulay
force-pushed
the
domain_api
branch
2 times, most recently
from
7d6ab3c to
d83a6be
Compare
Convenience method for domain.getValues().isAll() && !domain.isNullAllowed(). This translates to IS NOT NULL.
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
I assume this is still in progress / ready for review and merge @elonazoulay @martint @findepi |
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time. |
Convenience method for domain.getValues().isAll() && !domain.isNullAllowed(). This translates to IS NOT NULL.
Description
Additional context and related issues
This can be applied to the follow pr as well:
#16979
Release notes
(X ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: