Merge #31730

31730:  sql: unreserve MINVALUE and MAXVALUE r=knz a=knz

First two commits from  #31725 and  #31731.

The keywords MINVALUE and MAXVALUE had been marked as reserved away
from being a valid column name to accommodate the CockroachDB-specific
"PARTITIONING" syntax.

This is unfortunate because these two words are perfectly fine English
names for things people may want to have as attributes in their
relations.

This patch removes the limitation by unreserving the two keywords and
handling the special words in the planning code for the partitioning
feature. In every other context the two words are not treated
specially and handled like any possible column name / identifier.

Release note (sql change): It is now again possible to use the
keywords MINVALUE and MAXVALUE as column names, for compatibility with
PostgreSQL.

Co-authored-by: Raphael 'kena' Poss <[email protected]>

docs/generated/sql/bnf/stmt_block.bnf

@@ -371,7 +371,7 @@ on_conflict ::=
 	| 'ON' 'CONFLICT' opt_conf_expr 'DO' 'NOTHING'
 
 a_expr ::=
-	( c_expr | '+' a_expr | '-' a_expr | '~' a_expr | 'NOT' a_expr | 'NOT' a_expr | 'DEFAULT' | 'MAXVALUE' | 'MINVALUE' ) ( ( 'TYPECAST' cast_target | 'TYPEANNOTATE' typename | 'COLLATE' collation_name | '+' a_expr | '-' a_expr | '*' a_expr | '/' a_expr | 'FLOORDIV' a_expr | '%' a_expr | '^' a_expr | '#' a_expr | '&' a_expr | '|' a_expr | '<' a_expr | '>' a_expr | '?' a_expr | 'JSON_SOME_EXISTS' a_expr | 'JSON_ALL_EXISTS' a_expr | 'CONTAINS' a_expr | 'CONTAINED_BY' a_expr | '=' a_expr | 'CONCAT' a_expr | 'LSHIFT' a_expr | 'RSHIFT' a_expr | 'FETCHVAL' a_expr | 'FETCHTEXT' a_expr | 'FETCHVAL_PATH' a_expr | 'FETCHTEXT_PATH' a_expr | 'REMOVE_PATH' a_expr | 'INET_CONTAINED_BY_OR_EQUALS' a_expr | 'INET_CONTAINS_OR_CONTAINED_BY' a_expr | 'INET_CONTAINS_OR_EQUALS' a_expr | 'LESS_EQUALS' a_expr | 'GREATER_EQUALS' a_expr | 'NOT_EQUALS' a_expr | 'AND' a_expr | 'OR' a_expr | 'LIKE' a_expr | 'LIKE' a_expr 'ESCAPE' a_expr | 'NOT' 'LIKE' a_expr | 'NOT' 'LIKE' a_expr 'ESCAPE' a_expr | 'ILIKE' a_expr | 'ILIKE' a_expr 'ESCAPE' a_expr | 'NOT' 'ILIKE' a_expr | 'NOT' 'ILIKE' a_expr 'ESCAPE' a_expr | 'SIMILAR' 'TO' a_expr | 'SIMILAR' 'TO' a_expr 'ESCAPE' a_expr | 'NOT' 'SIMILAR' 'TO' a_expr | 'NOT' 'SIMILAR' 'TO' a_expr 'ESCAPE' a_expr | '~' a_expr | 'NOT_REGMATCH' a_expr | 'REGIMATCH' a_expr | 'NOT_REGIMATCH' a_expr | 'IS' 'NAN' | 'IS' 'NOT' 'NAN' | 'IS' 'NULL' | 'ISNULL' | 'IS' 'NOT' 'NULL' | 'NOTNULL' | 'IS' 'TRUE' | 'IS' 'NOT' 'TRUE' | 'IS' 'FALSE' | 'IS' 'NOT' 'FALSE' | 'IS' 'UNKNOWN' | 'IS' 'NOT' 'UNKNOWN' | 'IS' 'DISTINCT' 'FROM' a_expr | 'IS' 'NOT' 'DISTINCT' 'FROM' a_expr | 'IS' 'OF' '(' type_list ')' | 'IS' 'NOT' 'OF' '(' type_list ')' | 'BETWEEN' opt_asymmetric b_expr 'AND' a_expr | 'NOT' 'BETWEEN' opt_asymmetric b_expr 'AND' a_expr | 'BETWEEN' 'SYMMETRIC' b_expr 'AND' a_expr | 'NOT' 'BETWEEN' 'SYMMETRIC' b_expr 'AND' a_expr | 'IN' in_expr | 'NOT' 'IN' in_expr | subquery_op sub_type a_expr ) )*
+	( c_expr | '+' a_expr | '-' a_expr | '~' a_expr | 'NOT' a_expr | 'NOT' a_expr | 'DEFAULT' ) ( ( 'TYPECAST' cast_target | 'TYPEANNOTATE' typename | 'COLLATE' collation_name | '+' a_expr | '-' a_expr | '*' a_expr | '/' a_expr | 'FLOORDIV' a_expr | '%' a_expr | '^' a_expr | '#' a_expr | '&' a_expr | '|' a_expr | '<' a_expr | '>' a_expr | '?' a_expr | 'JSON_SOME_EXISTS' a_expr | 'JSON_ALL_EXISTS' a_expr | 'CONTAINS' a_expr | 'CONTAINED_BY' a_expr | '=' a_expr | 'CONCAT' a_expr | 'LSHIFT' a_expr | 'RSHIFT' a_expr | 'FETCHVAL' a_expr | 'FETCHTEXT' a_expr | 'FETCHVAL_PATH' a_expr | 'FETCHTEXT_PATH' a_expr | 'REMOVE_PATH' a_expr | 'INET_CONTAINED_BY_OR_EQUALS' a_expr | 'INET_CONTAINS_OR_CONTAINED_BY' a_expr | 'INET_CONTAINS_OR_EQUALS' a_expr | 'LESS_EQUALS' a_expr | 'GREATER_EQUALS' a_expr | 'NOT_EQUALS' a_expr | 'AND' a_expr | 'OR' a_expr | 'LIKE' a_expr | 'LIKE' a_expr 'ESCAPE' a_expr | 'NOT' 'LIKE' a_expr | 'NOT' 'LIKE' a_expr 'ESCAPE' a_expr | 'ILIKE' a_expr | 'ILIKE' a_expr 'ESCAPE' a_expr | 'NOT' 'ILIKE' a_expr | 'NOT' 'ILIKE' a_expr 'ESCAPE' a_expr | 'SIMILAR' 'TO' a_expr | 'SIMILAR' 'TO' a_expr 'ESCAPE' a_expr | 'NOT' 'SIMILAR' 'TO' a_expr | 'NOT' 'SIMILAR' 'TO' a_expr 'ESCAPE' a_expr | '~' a_expr | 'NOT_REGMATCH' a_expr | 'REGIMATCH' a_expr | 'NOT_REGIMATCH' a_expr | 'IS' 'NAN' | 'IS' 'NOT' 'NAN' | 'IS' 'NULL' | 'ISNULL' | 'IS' 'NOT' 'NULL' | 'NOTNULL' | 'IS' 'TRUE' | 'IS' 'NOT' 'TRUE' | 'IS' 'FALSE' | 'IS' 'NOT' 'FALSE' | 'IS' 'UNKNOWN' | 'IS' 'NOT' 'UNKNOWN' | 'IS' 'DISTINCT' 'FROM' a_expr | 'IS' 'NOT' 'DISTINCT' 'FROM' a_expr | 'IS' 'OF' '(' type_list ')' | 'IS' 'NOT' 'OF' '(' type_list ')' | 'BETWEEN' opt_asymmetric b_expr 'AND' a_expr | 'NOT' 'BETWEEN' opt_asymmetric b_expr 'AND' a_expr | 'BETWEEN' 'SYMMETRIC' b_expr 'AND' a_expr | 'NOT' 'BETWEEN' 'SYMMETRIC' b_expr 'AND' a_expr | 'IN' in_expr | 'NOT' 'IN' in_expr | subquery_op sub_type a_expr ) )*
 
 reset_session_stmt ::=
 	'RESET' session_var
@@ -622,7 +622,9 @@ unreserved_keyword ::=
 	| 'LOW'
 	| 'MATCH'
 	| 'MATERIALIZED'
+	| 'MAXVALUE'
 	| 'MINUTE'
+	| 'MINVALUE'
 	| 'MONTH'
 	| 'NAMES'
 	| 'NAN'
@@ -1457,8 +1459,6 @@ type_func_name_keyword ::=
 
 cockroachdb_extra_type_func_name_keyword ::=
 	'FAMILY'
-	| 'MAXVALUE'
-	| 'MINVALUE'
 
 cockroachdb_extra_reserved_keyword ::=
 	'INDEX'

pkg/ccl/partitionccl/partition.go

@@ -37,6 +37,13 @@ func valueEncodePartitionTuple(
 	maybeTuple tree.Expr,
 	cols []sqlbase.ColumnDescriptor,
 ) ([]byte, error) {
+	// Replace any occurrences of the MINVALUE/MAXVALUE pseudo-names
+	// into MinVal and MaxVal, to be recognized below.
+	// We are operating in a context where the expressions cannot
+	// refer to table columns, so these two names are unambiguously
+	// referring to the desired partition boundaries.
+	maybeTuple, _ = tree.WalkExpr(replaceMinMaxValVisitor{}, maybeTuple)
+
 	tuple, ok := maybeTuple.(*tree.Tuple)
 	if !ok {
 		// If we don't already have a tuple, promote whatever we have to a 1-tuple.
@@ -60,15 +67,15 @@ func valueEncodePartitionTuple(
 			value = encoding.EncodeNotNullValue(value, encoding.NoColumnID)
 			value = encoding.EncodeNonsortingUvarint(value, uint64(sqlbase.PartitionDefaultVal))
 			continue
-		case tree.MinVal:
+		case tree.PartitionMinVal:
 			if typ != tree.PartitionByRange {
 				return nil, errors.Errorf("%s cannot be used with PARTITION BY %s", expr, typ)
 			}
 			// NOT NULL is used to signal that a PartitionSpecialValCode follows.
 			value = encoding.EncodeNotNullValue(value, encoding.NoColumnID)
 			value = encoding.EncodeNonsortingUvarint(value, uint64(sqlbase.PartitionMinVal))
 			continue
-		case tree.MaxVal:
+		case tree.PartitionMaxVal:
 			if typ != tree.PartitionByRange {
 				return nil, errors.Errorf("%s cannot be used with PARTITION BY %s", expr, typ)
 			}
@@ -109,6 +116,28 @@ func valueEncodePartitionTuple(
 	return value, nil
 }
 
+// replaceMinMaxValVisitor replaces occurrences of the unqualified
+// identifiers "minvalue" and "maxvalue" in the partitioning
+// (sub-)exprs by the symbolic values tree.PartitionMinVal and
+// tree.PartitionMaxVal.
+type replaceMinMaxValVisitor struct{}
+
+// VisitPre satisfies the tree.Visitor interface.
+func (v replaceMinMaxValVisitor) VisitPre(expr tree.Expr) (recurse bool, newExpr tree.Expr) {
+	if t, ok := expr.(*tree.UnresolvedName); ok && t.NumParts == 1 {
+		switch t.Parts[0] {
+		case "minvalue":
+			return false, tree.PartitionMinVal{}
+		case "maxvalue":
+			return false, tree.PartitionMaxVal{}
+		}
+	}
+	return true, expr
+}
+
+// VisitPost satisfies the Visitor interface.
+func (replaceMinMaxValVisitor) VisitPost(expr tree.Expr) tree.Expr { return expr }
+
 func createPartitioningImpl(
 	ctx context.Context,
 	evalCtx *tree.EvalContext,

pkg/sql/parser/parse_test.go

@@ -198,7 +198,6 @@ func TestParse(t *testing.T) {
 		{`CREATE TABLE a (b INT CONSTRAINT c REFERENCES d)`},
 
 		{`CREATE TABLE a (b INT) PARTITION BY LIST (b) (PARTITION p1 VALUES IN (1, DEFAULT), PARTITION p2 VALUES IN ((1, 2), (3, 4)))`},
-		{`CREATE TABLE a (b INT) PARTITION BY RANGE (b) (PARTITION p1 VALUES FROM (MINVALUE) TO (1), PARTITION p2 VALUES FROM (2, MAXVALUE) TO (4, 4), PARTITION p3 VALUES FROM (4, 4) TO (MAXVALUE))`},
 		// This monstrosity was added on the assumption that it's more readable
 		// than all on one line. Feel free to rip it out if you come across it
 		// and disagree.
@@ -1850,10 +1849,19 @@ func TestParse2(t *testing.T) {
 			`CREATE TABLE "index" (x INT)`},
 		{`CREATE TABLE NOTHING (x INT)`,
 			`CREATE TABLE "nothing" (x INT)`},
+
+		// Ensure MINVALUE/MAXVALUE are not reserved.
 		{`CREATE TABLE MINVALUE (x INT)`,
-			`CREATE TABLE "minvalue" (x INT)`},
+			`CREATE TABLE minvalue (x INT)`},
 		{`CREATE TABLE MAXVALUE (x INT)`,
-			`CREATE TABLE "maxvalue" (x INT)`},
+			`CREATE TABLE maxvalue (x INT)`},
+		{`CREATE TABLE foo (MINVALUE INT)`,
+			`CREATE TABLE foo (minvalue INT)`},
+		{`CREATE TABLE foo (MAXVALUE INT)`,
+			`CREATE TABLE foo (maxvalue INT)`},
+
+		{`CREATE TABLE a (b INT) PARTITION BY RANGE (b) (PARTITION p1 VALUES FROM (MINVALUE) TO (1), PARTITION p2 VALUES FROM (2, MAXVALUE) TO (4, 4), PARTITION p3 VALUES FROM (4, 4) TO (MAXVALUE))`,
+			`CREATE TABLE a (b INT) PARTITION BY RANGE (b) (PARTITION p1 VALUES FROM (minvalue) TO (1), PARTITION p2 VALUES FROM (2, maxvalue) TO (4, 4), PARTITION p3 VALUES FROM (4, 4) TO (maxvalue))`},
 	}
 	for _, d := range testData {
 		stmts, err := parser.Parse(d.sql)

pkg/sql/parser/sql.y

@@ -6893,14 +6893,6 @@ a_expr:
   {
     $$.val = tree.DefaultVal{}
   }
-| MAXVALUE
-  {
-    $$.val = tree.MaxVal{}
-  }
-| MINVALUE
-  {
-    $$.val = tree.MinVal{}
-  }
 // The UNIQUE predicate is a standard SQL feature but not yet implemented
 // in PostgreSQL (as of 10.5).
 | UNIQUE '(' error { return unimplemented(sqllex, "UNIQUE predicate") }
@@ -8491,7 +8483,9 @@ unreserved_keyword:
 | LOW
 | MATCH
 | MATERIALIZED
+| MAXVALUE
 | MINUTE
+| MINVALUE
 | MONTH
 | NAMES
 | NAN
@@ -8717,11 +8711,8 @@ type_func_name_keyword:
 // Adding keywords here creates non-resolvable incompatibilities with
 // postgres clients.
 //
-// TODO(dan): Make MINVALUE/MAXVALUE less reserved.
 cockroachdb_extra_type_func_name_keyword:
   FAMILY
-| MAXVALUE
-| MINVALUE
 
 // Reserved keyword --- these keywords are usable only as a unrestricted_name.
 //

pkg/sql/sem/tree/expr.go

@@ -699,19 +699,19 @@ func (node DefaultVal) Format(ctx *FmtCtx) {
 // ResolvedType implements the TypedExpr interface.
 func (DefaultVal) ResolvedType() types.T { return nil }
 
-// MaxVal represents the MAXVALUE expression.
-type MaxVal struct{}
+// PartitionMaxVal represents the MAXVALUE expression.
+type PartitionMaxVal struct{}
 
 // Format implements the NodeFormatter interface.
-func (node MaxVal) Format(ctx *FmtCtx) {
+func (node PartitionMaxVal) Format(ctx *FmtCtx) {
 	ctx.WriteString("MAXVALUE")
 }
 
-// MinVal represents the MINVALUE expression.
-type MinVal struct{}
+// PartitionMinVal represents the MINVALUE expression.
+type PartitionMinVal struct{}
 
 // Format implements the NodeFormatter interface.
-func (node MinVal) Format(ctx *FmtCtx) {
+func (node PartitionMinVal) Format(ctx *FmtCtx) {
 	ctx.WriteString("MINVALUE")
 }
 
@@ -1700,8 +1700,8 @@ func (node *TupleStar) String() string        { return AsString(node) }
 func (node *AnnotateTypeExpr) String() string { return AsString(node) }
 func (node *UnaryExpr) String() string        { return AsString(node) }
 func (node DefaultVal) String() string        { return AsString(node) }
-func (node MaxVal) String() string            { return AsString(node) }
-func (node MinVal) String() string            { return AsString(node) }
+func (node PartitionMaxVal) String() string   { return AsString(node) }
+func (node PartitionMinVal) String() string   { return AsString(node) }
 func (node *Placeholder) String() string      { return AsString(node) }
 func (node dNull) String() string             { return AsString(node) }
 func (list *NameList) String() string         { return AsString(list) }