Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider updating Dedup definition to apply de-duplication to error values #126

Open
kasei opened this issue Oct 8, 2023 · 4 comments
Open
Labels
spec:bug Change fixing a bug in the specification (class 3) –see also spec:substantive

Comments

@kasei
Copy link
Contributor

kasei commented Oct 8, 2023

As discussed in #125, Dedup/Distinct currently does not handle the de-deduplication of error values which is important for handling COUNT(DISTINCT *) . This case could be special-cased in the text, but custom set functions. I'd like to see Dedup updated to handle error values natively as it allows custom aggregates to work over error values while having (what I think of as the expected) DISTINCT handling handled before the aggregation.

@hartig
Copy link
Contributor

hartig commented Oct 8, 2023

As I just wrote in #125 (comment), the question of error values is irrelevant for COUNT(DISTINCT *) (and also for COUNT(*)) because these are special cases for which the ListEval function is not even used. Instead, the expected result for these two special cases is defined purely based on the given sequence Ψ of solution mappings, rather than on the sequence M(Ψ) of lists as produced by ListEval.

@kasei
Copy link
Contributor Author

kasei commented Oct 8, 2023

You're right, ListEval isn't used in the case of COUNT(DISTINCT *). Including it in the discussion here is a mistake. I think the example you gave in #125 is a good summary of the issue using some arbitrary SomeSetFct set function.

@pfps
Copy link
Contributor

pfps commented Nov 20, 2024

Is this issue then resolved by #125 being merged?

@pfps pfps added the spec:bug Change fixing a bug in the specification (class 3) –see also spec:substantive label Nov 20, 2024
@hartig
Copy link
Contributor

hartig commented Nov 20, 2024

Yes, I would say the issue is resolved. @kasei ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:bug Change fixing a bug in the specification (class 3) –see also spec:substantive
Projects
None yet
Development

No branches or pull requests

3 participants