sql: fix bug where some session vars couldn't be set to "on" or "off"

[rohany]

Session variables were incorrectly erroring out on
string inputs even though string values of "on" and
"off" are accepted.

Fixes #46161.

Release justification: bug fix
Release note (bug fix): This PR fixes a bug where various session
variables whose value would display as "on" or "off" could not be
set to the values "on" or "off", only true or false.

[cockroach-teamcity]

This change is 

[knz]

Sorry I don't understand the code change. Why was the previous code incorrect? Can you walk me through the analysis step by step?

Reviewed 2 of 2 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained

[rohany]

all vars that had a GetStringVal function equal to makeBoolGetStringVal would have GetStringVal called on the input datum before Set was called. makeBoolGetStringVal would call into getSingleBool on the input datum, and would error out if the input datum wasn't a DBool. That's incorrect, because we allow for DString datums in these variables, as long as they are equal to on or off, which the Set function for these vars handles.

[knz]

Thanks. Maybe would have helped to add this explanation to the commit message.

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

---

pkg/sql/vars.go, line 902 at r1 (raw file):

}

func makeBoolGetStringValFn(varName string) getStringValFn {

Please spell out the semantics of this function in a docstring above it.

---

pkg/sql/vars.go, line 1040 at r1 (raw file):

}()

func getSingleBool(name string, val tree.Datum) (*tree.DBool, error) {

ditto

[rohany]

Updated. I think this should be backported, what do you think?

[awoods187]

+1 on backport

[knz]

yes to backport

Reviewed 1 of 1 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained

[RaduBerinde]

Thanks!

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

---

pkg/sql/logictest/testdata/logic_test/set, line 336 at r3 (raw file):

# Ensure that we can set variables to on/off.
statement ok
SET enable_zigzag_join = 'on';

If we want true and false to work as well, we should add those tests too.

[rohany]

bors r+

[rohany]

bors r-

[craig]

Canceled

[rohany]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)