Merge #46932

46932: settings: return valid values when setting an invalid cluster setting r=knz a=asubiotto ``` [email protected]:61456/movr> SET CLUSTER SETTING sql.defaults.vectorize=5; ERROR: invalid integer value '5' for enum setting HINT: Available values: 0: off, 1: auto, 2: on [email protected]:61456/movr> SET CLUSTER SETTING sql.defaults.vectorize='boom'; ERROR: invalid string value 'boom' for enum setting HINT: Available values: 0: off, 1: auto, 2: on ``` Release note (sql change): If setting an invalid cluster setting, the invalid values are now returned. Closes #35477 Co-authored-by: Alfonso Subiotto Marques <[email protected]>
cockroachdb · Apr 3, 2020 · 0c1b121 · 0c1b121
2 parents c36f7cb + 5516d84
commit 0c1b121
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 2 deletions.
diff --git a/pkg/settings/enum.go b/pkg/settings/enum.go
@@ -59,6 +59,24 @@ func (e *EnumSetting) ParseEnum(raw string) (int64, bool) {
 	return v, ok
 }
 
+// GetAvailableValuesAsHint returns the possible enum settings as a string that
+// can be provided as an error hint to a user.
+func (e *EnumSetting) GetAvailableValuesAsHint() string {
+	// First stabilize output by sorting by key.
+	valIdxs := make([]int, 0, len(e.enumValues))
+	for i := range e.enumValues {
+		valIdxs = append(valIdxs, int(i))
+	}
+	sort.Ints(valIdxs)
+
+	// Now use those indices
+	vals := make([]string, 0, len(e.enumValues))
+	for _, enumIdx := range valIdxs {
+		vals = append(vals, fmt.Sprintf("%d: %s", enumIdx, e.enumValues[int64(enumIdx)]))
+	}
+	return "Available values: " + strings.Join(vals, ", ")
+}
+
 func (e *EnumSetting) set(sv *Values, k int64) error {
 	if _, ok := e.enumValues[k]; !ok {
 		return errors.Errorf("unrecognized value %d", k)

diff --git a/pkg/sql/set_cluster_setting.go b/pkg/sql/set_cluster_setting.go
@@ -290,14 +290,14 @@ func toSettingString(
 			if ok {
 				return settings.EncodeInt(v), nil
 			}
-			return "", errors.Errorf("invalid integer value '%d' for enum setting", *i)
+			return "", errors.WithHintf(errors.Errorf("invalid integer value '%d' for enum setting", *i), setting.GetAvailableValuesAsHint())
 		} else if s, ok := d.(*tree.DString); ok {
 			str := string(*s)
 			v, ok := setting.ParseEnum(str)
 			if ok {
 				return settings.EncodeInt(v), nil
 			}
-			return "", errors.Errorf("invalid string value '%s' for enum setting", str)
+			return "", errors.WithHintf(errors.Errorf("invalid string value '%s' for enum setting", str), setting.GetAvailableValuesAsHint())
 		}
 		return "", errors.Errorf("cannot use %s %T value for enum setting, must be int or string", d.ResolvedType(), d)
 	case *settings.ByteSizeSetting: