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.

Sign up for GitHub

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

Jump to bottom

sql: equality, IN incorrect when dealing with "deep" tuples #22717

Closed
RaduBerinde opened this issue Feb 14, 2018 · 8 comments
Closed

sql: equality, IN incorrect when dealing with "deep" tuples #22717

RaduBerinde opened this issue Feb 14, 2018 · 8 comments
Labels
A-sql-execution Relating to SQL execution. A-sql-optimizer SQL logical planning and optimizations. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.
Milestone
2.1

Comments

@RaduBerinde
Copy link
Member

SELECT ((1, 1), (2, NULL)) = ((1, 1), (2, NULL)) and SELECT ((1, 1), (2, NULL)) IN (((1, 1), (2, NULL))) return true instead of NULL.

See #22670 (comment)
@RaduBerinde RaduBerinde mentioned this issue Feb 14, 2018
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Feb 14, 2018
@RaduBerinde
sql: fix NULL handling in tuple IN expressions
ffacee9 
This change fixes handling of NULLs with IN expressions. The following
examples illustrate the interesting cases that are fixed:
```
  NULL IN <empty-tuple>       = false
  (1, NULL) IN <empty-tuple>  = false
  (1, NULL) IN ((1, NULL))    = NULL
  (1, NULL) IN ((1, 1))       = NULL
  (1, NULL) IN ((2, NULL))    = false
```

To implement this correctly, we now require that only tuples that
don't contain NULLs can be marked as sorted. This is because we can't
use binary search to, for example "match" `(1, NULL)` with `(1, 1)`
(necessary to know if the result is false or NULL).

Note that equality is broken for deeper tuples (cockroachdb#22717), and the new
IN code relies on the equality code so it is broken for deep tuples as
well.

Updates cockroachdb#22670.

Release note: None
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Feb 15, 2018
@RaduBerinde
sql: fix NULL handling in tuple IN expressions
2db0762 
This change fixes handling of NULLs with IN expressions. The following
examples illustrate the interesting cases that are fixed:
```
  NULL IN <empty-tuple>       = false
  (1, NULL) IN <empty-tuple>  = false
  (1, NULL) IN ((1, NULL))    = NULL
  (1, NULL) IN ((1, 1))       = NULL
  (1, NULL) IN ((2, NULL))    = false
```

To implement this correctly, we now require that only tuples that
don't contain NULLs can be marked as sorted. This is because we can't
use binary search to, for example "match" `(1, NULL)` with `(1, 1)`
(necessary to know if the result is false or NULL).

Note that equality is broken for deeper tuples (cockroachdb#22717), and the new
IN code relies on the equality code so it is broken for deep tuples as
well.

Updates cockroachdb#22670.

Release note (bug fix): Fixed incorrect evaluation of IN expressions
where the left-hand side is a tuple, and some of the tuples on either
side contain NULL.
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Feb 15, 2018
@RaduBerinde
sql: fix NULL handling in tuple IN expressions
bd6b5ea 
This change fixes handling of NULLs with IN expressions. The following
examples illustrate the interesting cases that are fixed:
```
  NULL IN <empty-tuple>       = false
  (1, NULL) IN <empty-tuple>  = false
  (1, NULL) IN ((1, NULL))    = NULL
  (1, NULL) IN ((1, 1))       = NULL
  (1, NULL) IN ((2, NULL))    = false
```

To implement this correctly, we now require that only tuples that
don't contain NULLs can be marked as sorted. This is because we can't
use binary search to, for example "match" `(1, NULL)` with `(1, 1)`
(necessary to know if the result is false or NULL).

Note that equality is broken for deeper tuples (cockroachdb#22717), and the new
IN code relies on the equality code so it is broken for deep tuples as
well.

Updates cockroachdb#22670.

Release note (bug fix): Fixed incorrect evaluation of IN expressions
where the left-hand side is a tuple, and some of the tuples on either
side contain NULL.
@petermattis petermattis added this to the 2.1 milestone Feb 26, 2018
@knz knz added A-sql-optimizer SQL logical planning and optimizations. S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. A-sql-execution Relating to SQL execution. labels May 14, 2018
@knz
Copy link
Contributor

knz commented May 14, 2018

@RaduBerinde I think this issue still exists right?

@RaduBerinde
Copy link
Member Author

Yes.

@jordanlewis
Copy link
Member

Wait, but to be clear, we've decided that this is also a bug in Postgres, right?

This doesn't seem like an issue that people are going to notice in practice.

@jordanlewis
Copy link
Member

(notice or care about)

@jordanlewis jordanlewis added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. and removed S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. labels May 24, 2018
@RaduBerinde
Copy link
Member Author

Yes, it is also in PG. I agree it's probably not very important.

@jordanlewis jordanlewis assigned knz and unassigned rjnn Aug 21, 2018
@jordanlewis
Copy link
Member

Reassigning to @knz since he knows about this. Feel free to close this if you think it's not important.

@knz
Copy link
Contributor

knz commented Aug 21, 2018

I was addressing this #27885. Will look further.

@nstewart nstewart added the S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases. label Sep 18, 2018
@knz knz removed their assignment Aug 26, 2019
@jordanlewis
Copy link
Member

Closing this - the first one works as expected, and the second one is identical to Postgres.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-sql-execution Relating to SQL execution. A-sql-optimizer SQL logical planning and optimizations. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.
Projects
None yet
Milestone
2.1
Development

No branches or pull requests
6 participants
@jordanlewis @knz @nstewart @rjnn @petermattis @RaduBerinde