sql: allow comparisons of tuples

[maddyblue]

Fixes #6206

---

This change is

[andreimatei]

This is not yet used by index selection, is it? (and in that case I don't think we can say it fixes the bug)

---

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

sql/parser/eval.go, line 744 [r1] (raw file):
isn't there a DTuple.Compare directly?

---

Comments from Reviewable

[maddyblue]

Changed to "See".

---

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

sql/parser/eval.go, line 744 [r1] (raw file):
DTuple.Compare has different semantics than this. For example it allows comparing tuples of different lengths. However we can handle those and then call it directly, yes.

---

Comments from Reviewable

[maddyblue]

Updated with tests for varying sizes and types. We now have an unfortunate panic/recover because Datum.Compare's function signature returns just an int, no errors, and it panics on type mismatch errors. Calling defer on every row where a tuple is being compared is perhaps a big enough performance problem that we should 1) not use DTuple's Compare method or 2) change the function signature of Datum.Compare to also return an error. Thoughts?

---

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

Comments from Reviewable

[RaduBerinde]

LGTM, but the issue should stay open until we generate constraints/spans in index selection. Otherwise you could argue it's worse than before, when the user was forced to rewrite the expression but when they did the query didn't scan the entire table..

---

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

Comments from Reviewable

[petermattis]

---

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

sql/parser/eval.go, line 770 [r3] (raw file):
This should never happen as type-checking will not allow comparison of tuples with unequal numbers of elements. It's probably fine to leave out this check and rely on type-checking to catch unequal tuple lengths.

---

sql/parser/eval.go, line 773 [r3] (raw file):
Is this necessary? Seems like the panic in DTuple.Compare shouldn't be reachable from here if we've already performed type-checking. If this is possible, can you add a test case?

---

Comments from Reviewable

[maddyblue]

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

sql/parser/eval.go, line 770 [r3] (raw file):
What you describe doesn't seem to be the case. See the test case below with this error message. When this length check is removed that statement succeeds instead of errors.

---

sql/parser/eval.go, line 773 [r3] (raw file):
Yes, it's necessary, in the case of comparing tuples of the same length that have members that are different types. See the test case below for unsupported comparison.

---

Comments from Reviewable

[petermattis]

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

sql/parser/eval.go, line 770 [r3] (raw file):
That's surprising. Why isn't this being caught by typeCheckTupleEQ.

---

sql/parser/eval.go, line 773 [r3] (raw file):
See my other comment. I would think this would be caught by typeCheckTupleEQ. Do you mind investigating why it isn't? Are we not running type-checking in some cases?

---

Comments from Reviewable

[petermattis]

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

sql/parser/eval.go, line 773 [r3] (raw file):
Ah, typeCheckTupleEQ is only checking tuples during equality. I think this change needs to change typeCheckComparisonOp to check other tuple operations.

---

Comments from Reviewable

[maddyblue]

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

sql/parser/eval.go, line 770 [r3] (raw file):
Done.

---

sql/parser/eval.go, line 773 [r3] (raw file):
Done.

---

Comments from Reviewable

[petermattis]

---

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

sql/testdata/tuple, line 85 [r4] (raw file):
See below about the error message. This should be <int> > <string>, or even better: (<int>, <int>) > (<int>, <string>). But let's just file an issue and fix it in a separate PR.

---

sql/testdata/tuple, line 88 [r4] (raw file):
Shouldn't that be 2, 3? Let's file a bug about this incorrect error message. I think it is because we don't actually have a > operator, but implement it via < and swapping the arguments.

---

Comments from Reviewable