Skip to content

Support for removing Universal Set#44

Open
mineralntl wants to merge 2 commits into
mainfrom
task/redoUniversalSetRemoval
Open

Support for removing Universal Set#44
mineralntl wants to merge 2 commits into
mainfrom
task/redoUniversalSetRemoval

Conversation

@mineralntl
Copy link
Copy Markdown
Collaborator

@mineralntl mineralntl commented Sep 27, 2024

Redoing this as it fell thru the cracks many many moons ago - this is in support for NationalSecurityAgency/datawave#1814

Comment on lines 96 to +97
Assertions.assertEquals(Sets.newHashSet("rowA", "rowB", "rowC"), helper.getAllFields(null));
Assertions.assertEquals(Sets.newHashSet("rowA", "rowB", "rowC"), helper.getAllFields(Collections.emptySet()));
Assertions.assertEquals(Collections.emptySet(), helper.getAllFields(Collections.emptySet()));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought a null datatype filter and an empty datatype filter both implied "no filter, return everything". Why then the change in behavior?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original intent was for a null datatype filter to allow all the things, a populated filter to allow only things contained therein, and an empty filter to allow none of the things.

Copy link
Copy Markdown
Collaborator

@hlgp hlgp Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the null vs empty meaning all or none nuance came up again this week for a related area of code. maybe we need some explicit enums up in here to propagate with the decision made by one paradigm in one place with the rest of the code just dealing with ALL, NONE, or LIST

@mineralntl mineralntl force-pushed the task/redoUniversalSetRemoval branch from 4b4a202 to c063ec2 Compare October 23, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants