Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Uzair5162)

pkg/sql/sem/tree/datum.go line 119 at r1 (raw file):

	// Compare returns -1 if the receiver is less than other, 0 if receiver is
	// equal to other and +1 if receiver is greater than 'other'. Compare is safe
	// to use with a nil eval.Context.

[nit] mention that a nil eval.Context results in default behavior.

Also, there is another case where nil eval.Context isn't valid (and shouldn't be) - tree.Placeholder. Let's call that out in the comment.

pkg/sql/sem/eval/compare_test.go line 28 at r1 (raw file):

// Test that we can compare random datums with a nil eval context.
func TestNilEvalContextCompareRandom(t *testing.T) {

[nit] Let's move this test to datum_test.go, since it's already being used to test datum comparison.

pkg/sql/sem/eval/compare_test.go line 39 at r1 (raw file):

		typ := randgen.RandType(rng)
		left := randgen.RandDatum(rng, typ, false)
		right := randgen.RandDatum(rng, typ, false)

Let's pass true here to allow NULL values. You can often catch edge cases that way.

[nit] Usually, when passing an unnamed value as a parameter like that, we also add an inline comment with the parameter name, like this:

right := randgen.RandDatum(rng, typ, false /* nullOk */)

pkg/sql/sem/eval/compare_test.go line 53 at r1 (raw file):

// Test that we can compare two equal TimeTZ datums with a nil eval context.
func TestNilEvalContextCompareTimeTZ(t *testing.T) {

There's already a TestCompareTimestamps test in datum_test.go - instead of making this new test, let's modify that one to run also run Compare with a nil eval.Context.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)

pkg/sql/sem/tree/datum.go line 119 at r1 (raw file):

Done. Good find!

pkg/sql/sem/eval/compare_test.go line 28 at r1 (raw file):

datum_test.go is in the tree pkg which creates cyclic dependencies with the eval pkg needed for eval.Context :(

pkg/sql/sem/eval/compare_test.go line 39 at r1 (raw file):

Done.

Nice work!

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Uzair5162)

pkg/sql/sem/eval/compare_test.go line 44 at r2 (raw file):

		// Also test comparing a random datum to itself with a nil eval context.
		right = left

[nit] I think it's more clear if you just do left.Compare(..., left). Is there a reason we can't do that?

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @DrewKimball)

pkg/sql/sem/eval/compare_test.go line 44 at r2 (raw file):

Nope, done.