Prechádzať zdrojové kódy

fix jsonb numeric patch values and aggregate exact comparisons

Danilo Fragoso 2 dní pred
rodič
commit
cd26258e3c

+ 1 - 1
pkg/capabilities/capabilities.go

@@ -237,7 +237,7 @@ func Current() *Document {
 			JSONB: JSONB{
 			JSONB: JSONB{
 				Canonicalization: "canonical JSON text: object keys sorted byte-wise, duplicate object keys last-wins, no insignificant whitespace, numbers exact normalized decimals (1.50 becomes 1.5, 1e2 becomes 100; no exponent, no trailing fractional zeros, negative zero becomes 0); canonicalization is idempotent and every write path stores the canonical bytes, so a reload reads back the same text",
 				Canonicalization: "canonical JSON text: object keys sorted byte-wise, duplicate object keys last-wins, no insignificant whitespace, numbers exact normalized decimals (1.50 becomes 1.5, 1e2 becomes 100; no exponent, no trailing fractional zeros, negative zero becomes 0); canonicalization is idempotent and every write path stores the canonical bytes, so a reload reads back the same text",
 				Ordering:         "jsonb equality and comparison operators compare exact canonical text bytes, so key order and number spelling are irrelevant but 1 and 1.0000000000000000001 are distinct and integers above 2^53 are not rounded; DISTINCT and GROUP BY use the canonical value (JSONB columns are canonicalized on read too); ORDER BY follows the engine's existing value comparison on canonical text, where both-numeric pairs compare numerically and every other pair compares byte-wise",
 				Ordering:         "jsonb equality and comparison operators compare exact canonical text bytes, so key order and number spelling are irrelevant but 1 and 1.0000000000000000001 are distinct and integers above 2^53 are not rounded; DISTINCT and GROUP BY use the canonical value (JSONB columns are canonicalized on read too); ORDER BY follows the engine's existing value comparison on canonical text, where both-numeric pairs compare numerically and every other pair compares byte-wise",
-				Operators:        Feature{Supported: true, Status: "supported", SQLState: "42804", Notes: "-> and ->> extract a member/element ($ path also accepted); #> and #>> extract by a text[] path (or a $ path); @> and <@ test containment with PostgreSQL's object/array/scalar rules; ? tests an object key or top-level string array element; ?| and ?& test a text[] of keys; || merges objects, concatenates arrays, and wraps non-arrays as single-element arrays; - deletes an object key, array index, or text[] of keys/elements; #- deletes a path; NULL operands return NULL, missing keys return SQL NULL (a JSON null member is a jsonb 'null' for -> and SQL NULL for ->>), and @> sees a JSON null member as a value; array containment is duplicate-insensitive (each candidate element only needs some matching container element, as in PostgreSQL); a text-only @> or <@ is rejected with 42804; - with an integer on an object reports 22023; the HTTP /query API substitutes ? textually, so a statement using the ? operator cannot also use ? positional parameters there"},
+				Operators:        Feature{Supported: true, Status: "supported", SQLState: "42804", Notes: "-> and ->> extract a member/element ($ path also accepted); #> and #>> extract by a text[] path (or a $ path); @> and <@ test containment with PostgreSQL's object/array/scalar rules; ? tests an object key or top-level string array element (a scalar string never matches); ?| and ?& test a text[] of keys; || merges objects, concatenates arrays, and wraps non-arrays as single-element arrays; - deletes an object key, array index, or text[] of keys/elements; #- deletes a path; NULL operands return NULL, missing keys return SQL NULL (a JSON null member is a jsonb 'null' for -> and SQL NULL for ->>), and @> sees a JSON null member as a value; array containment is duplicate-insensitive (each candidate element only needs some matching container element, as in PostgreSQL); a text-only @> or <@ is rejected with 42804; - with an integer on an object reports 22023; the HTTP /query API substitutes ? textually, so a statement using the ? operator cannot also use ? positional parameters there"},
 				Functions:        Feature{Supported: true, Status: "supported", SQLState: "22023", Notes: "jsonb_typeof, jsonb_array_length, jsonb_strip_nulls, jsonb_pretty, jsonb_build_object, jsonb_build_array, to_jsonb, jsonb_set, jsonb_insert; jsonb_set with an empty path leaves the target unchanged, jsonb_set appends a missing array position and clamps a too-low index when create_if_missing is true, jsonb_insert clamps and appends array positions, and jsonb_insert onto an existing object key reports 22023 (cannot replace existing key); the JSON1 jsonb_* aliases (jsonb_set, jsonb_insert, jsonb_replace, jsonb_remove with $ paths) keep their previous value behavior and return canonical jsonb; wrong document kinds report 22023 and malformed input 22P02; the JSON (text) and JSON1 function surface is unchanged"},
 				Functions:        Feature{Supported: true, Status: "supported", SQLState: "22023", Notes: "jsonb_typeof, jsonb_array_length, jsonb_strip_nulls, jsonb_pretty, jsonb_build_object, jsonb_build_array, to_jsonb, jsonb_set, jsonb_insert; jsonb_set with an empty path leaves the target unchanged, jsonb_set appends a missing array position and clamps a too-low index when create_if_missing is true, jsonb_insert clamps and appends array positions, and jsonb_insert onto an existing object key reports 22023 (cannot replace existing key); the JSON1 jsonb_* aliases (jsonb_set, jsonb_insert, jsonb_replace, jsonb_remove with $ paths) keep their previous value behavior and return canonical jsonb; wrong document kinds report 22023 and malformed input 22P02; the JSON (text) and JSON1 function surface is unchanged"},
 				SetReturning:     Feature{Supported: true, Status: "single_from_item", SQLState: "0A000", Notes: "jsonb_each, jsonb_each_text, jsonb_array_elements, jsonb_array_elements_text, and jsonb_object_keys are supported only as the single FROM item (no joins); calling one in a projection is rejected with 0A000; a NULL document produces no rows"},
 				SetReturning:     Feature{Supported: true, Status: "single_from_item", SQLState: "0A000", Notes: "jsonb_each, jsonb_each_text, jsonb_array_elements, jsonb_array_elements_text, and jsonb_object_keys are supported only as the single FROM item (no joins); calling one in a projection is rejected with 0A000; a NULL document produces no rows"},
 				Parameters:       Feature{Supported: true, Status: "text_and_binary", SQLState: "22P02", Notes: "text-format OID 3802 parameters are canonicalized at Bind; PostgreSQL binary jsonb parameters (version byte 0x01 followed by the document text, which pgx sends by default) are accepted and canonicalized, while a wrong version or malformed body is 22P02; an invalid document fails at Bind and operators see a real jsonb value; OID 114 (JSON) behavior is unchanged"},
 				Parameters:       Feature{Supported: true, Status: "text_and_binary", SQLState: "22P02", Notes: "text-format OID 3802 parameters are canonicalized at Bind; PostgreSQL binary jsonb parameters (version byte 0x01 followed by the document text, which pgx sends by default) are accepted and canonicalized, while a wrong version or malformed body is 22P02; an invalid document fails at Bind and operators see a real jsonb value; OID 114 (JSON) behavior is unchanged"},

+ 1 - 1
pkg/capabilities/capabilities_test.go

@@ -256,7 +256,7 @@ func TestJSONBIsAdvertised(t *testing.T) {
 	if !jsonb.Operators.Supported || jsonb.Operators.SQLState != pgerror.CodeDatatypeMismatch {
 	if !jsonb.Operators.Supported || jsonb.Operators.SQLState != pgerror.CodeDatatypeMismatch {
 		t.Fatalf("json.jsonb.operators = %+v", jsonb.Operators)
 		t.Fatalf("json.jsonb.operators = %+v", jsonb.Operators)
 	}
 	}
-	for _, want := range []string{"@>", "<@", "?|", "?&", "#>", "#>>", "#-", "||", "SQL NULL"} {
+	for _, want := range []string{"@>", "<@", "?|", "?&", "#>", "#>>", "#-", "||", "scalar string never matches", "SQL NULL"} {
 		if !strings.Contains(jsonb.Operators.Notes, want) {
 		if !strings.Contains(jsonb.Operators.Notes, want) {
 			t.Errorf("json.jsonb.operators notes do not mention %q", want)
 			t.Errorf("json.jsonb.operators notes do not mention %q", want)
 		}
 		}

+ 16 - 2
pkg/executor/executor.go

@@ -7186,7 +7186,13 @@ func (e *Executor) evalExprWithAggregates(expr parser.Expr, rows []storage.Row)
 			condVal, _ := e.evalExprWithAggregates(when.Condition, rows)
 			condVal, _ := e.evalExprWithAggregates(when.Condition, rows)
 			var matched bool
 			var matched bool
 			if ex.Operand != nil {
 			if ex.Operand != nil {
-				matched = operand != nil && condVal != nil && compare(operand, condVal) == 0
+				if operand != nil && condVal != nil {
+					equal, cmpErr := e.compareExprValues(lexer.TokenEq, ex.Operand, when.Condition, operand, condVal)
+					if cmpErr != nil {
+						return nil, cmpErr
+					}
+					matched = equal
+				}
 			} else {
 			} else {
 				matched = toBool(condVal)
 				matched = toBool(condVal)
 			}
 			}
@@ -7217,6 +7223,10 @@ func (e *Executor) evalExprWithAggregates(expr parser.Expr, rows []storage.Row)
 		if err != nil {
 		if err != nil {
 			return nil, err
 			return nil, err
 		}
 		}
+		left, right, err = e.canonicalizeArrayComparison(ex.Left, ex.Right, left, right)
+		if err != nil {
+			return nil, err
+		}
 		distinct := false
 		distinct := false
 		switch {
 		switch {
 		case left == nil && right == nil:
 		case left == nil && right == nil:
@@ -7224,7 +7234,11 @@ func (e *Executor) evalExprWithAggregates(expr parser.Expr, rows []storage.Row)
 		case left == nil || right == nil:
 		case left == nil || right == nil:
 			distinct = true
 			distinct = true
 		default:
 		default:
-			distinct = compare(left, right) != 0
+			equal, cmpErr := e.compareExprValues(lexer.TokenEq, ex.Left, ex.Right, left, right)
+			if cmpErr != nil {
+				return nil, cmpErr
+			}
+			distinct = !equal
 		}
 		}
 		if ex.Not {
 		if ex.Not {
 			return !distinct, nil
 			return !distinct, nil

+ 21 - 4
pkg/executor/jsonb.go

@@ -765,10 +765,9 @@ func jsonbKeyExists(left, right interface{}) (interface{}, error) {
 			}
 			}
 		}
 		}
 		return false, nil
 		return false, nil
-	case string:
-		// A scalar JSON string counts as the key itself: '"x"' ? 'x' is true.
-		return value == key, nil
 	default:
 	default:
+		// Scalar strings never match: jsonb_exists only looks at object keys and
+		// top-level string array elements.
 		return false, nil
 		return false, nil
 	}
 	}
 }
 }
@@ -1572,6 +1571,24 @@ func (e *Executor) toJSONBNode(expr parser.Expr, value interface{}) (interface{}
 	return jsonbNodeFromSQLValue(value)
 	return jsonbNodeFromSQLValue(value)
 }
 }
 
 
+// jsonbPatchNode converts a JSON1 $ path patch value to a JSON tree node. A
+// float64 must become a json.Number before it reaches the encoder, and NaN and
+// infinity are rejected instead of producing invalid JSON.
+func jsonbPatchNode(value interface{}) (interface{}, error) {
+	switch typed := value.(type) {
+	case jsonText:
+		if node, err := jsonDecode(string(typed)); err == nil {
+			return node, nil
+		}
+		return string(typed), nil
+	case nil, bool, int, int8, int16, int32, int64,
+		uint, uint32, uint64, float32, float64, string, []byte:
+		return jsonbNodeFromSQLValue(value)
+	default:
+		return jsonArgToNode(value), nil
+	}
+}
+
 // evalJSONBPatch evaluates the jsonb_set/jsonb_insert/jsonb_replace/
 // evalJSONBPatch evaluates the jsonb_set/jsonb_insert/jsonb_replace/
 // jsonb_remove family. A $ path keeps the JSON1 semantics the engine already
 // jsonb_remove family. A $ path keeps the JSON1 semantics the engine already
 // exposed; a PostgreSQL text[] path uses PostgreSQL's jsonb_set/jsonb_insert
 // exposed; a PostgreSQL text[] path uses PostgreSQL's jsonb_set/jsonb_insert
@@ -1651,7 +1668,7 @@ func (e *Executor) evalJSONBPatch(name string, fn *parser.FunctionCall, args []i
 			if e.isJSONBExpr(functionArgAt(fn, i+1)) {
 			if e.isJSONBExpr(functionArgAt(fn, i+1)) {
 				patchValue, err = jsonbNodeFromArg(args[i+1])
 				patchValue, err = jsonbNodeFromArg(args[i+1])
 			} else {
 			} else {
-				patchValue = jsonArgToNode(args[i+1])
+				patchValue, err = jsonbPatchNode(args[i+1])
 			}
 			}
 			if err != nil {
 			if err != nil {
 				return nil, err
 				return nil, err

+ 56 - 1
pkg/executor/jsonb_test.go

@@ -249,7 +249,7 @@ func TestJSONBContainmentAndExistence(t *testing.T) {
 		{`SELECT '{"a":1}'::jsonb ? 'b'`, false},
 		{`SELECT '{"a":1}'::jsonb ? 'b'`, false},
 		{`SELECT '[1,"x"]'::jsonb ? 'x'`, true},
 		{`SELECT '[1,"x"]'::jsonb ? 'x'`, true},
 		{`SELECT '[1,"x"]'::jsonb ? '1'`, false},
 		{`SELECT '[1,"x"]'::jsonb ? '1'`, false},
-		{`SELECT '"x"'::jsonb ? 'x'`, true},
+		{`SELECT '"x"'::jsonb ? 'x'`, false},
 		{`SELECT 'null'::jsonb ? 'x'`, false},
 		{`SELECT 'null'::jsonb ? 'x'`, false},
 		{`SELECT '{"a":1,"b":2}'::jsonb ?| '{b,c}'`, true},
 		{`SELECT '{"a":1,"b":2}'::jsonb ?| '{b,c}'`, true},
 		{`SELECT '{"a":1,"b":2}'::jsonb ?| '{c,d}'`, false},
 		{`SELECT '{"a":1,"b":2}'::jsonb ?| '{c,d}'`, false},
@@ -367,6 +367,9 @@ func TestJSONBScalarFunctions(t *testing.T) {
 		{`SELECT jsonb_set('{"a":1}', '$.b', '2')`, `{"a":1,"b":"2"}`},
 		{`SELECT jsonb_set('{"a":1}', '$.b', '2')`, `{"a":1,"b":"2"}`},
 		{`SELECT jsonb_set('{"a":1}', '$.a', '2')`, `{"a":"2"}`},
 		{`SELECT jsonb_set('{"a":1}', '$.a', '2')`, `{"a":"2"}`},
 		{`SELECT jsonb_set('{"a":1}', '$.b', '{"x":1}'::jsonb)`, `{"a":1,"b":{"x":1}}`},
 		{`SELECT jsonb_set('{"a":1}', '$.b', '{"x":1}'::jsonb)`, `{"a":1,"b":{"x":1}}`},
+		{`SELECT jsonb_set('{"a":1}', '$.b', 1.5)`, `{"a":1,"b":1.5}`},
+		{`SELECT jsonb_insert('{}', '$.b', 2.5)`, `{"b":2.5}`},
+		{`SELECT jsonb_replace('{"b":0}', '$.b', 2.5)`, `{"b":2.5}`},
 		{`SELECT jsonb_insert('{"a":1}', '$.b', '2')`, `{"a":1,"b":"2"}`},
 		{`SELECT jsonb_insert('{"a":1}', '$.b', '2')`, `{"a":1,"b":"2"}`},
 		{`SELECT jsonb_replace('{"a":1}', '$.a', '2')`, `{"a":"2"}`},
 		{`SELECT jsonb_replace('{"a":1}', '$.a', '2')`, `{"a":"2"}`},
 		{`SELECT jsonb_remove('{"a":1,"b":2}', '$.a')`, `{"b":2}`},
 		{`SELECT jsonb_remove('{"a":1,"b":2}', '$.a')`, `{"b":2}`},
@@ -907,3 +910,55 @@ func TestJSONBTextSetReturningTypesOnNull(t *testing.T) {
 		t.Fatalf("jsonb_array_elements_text(NULL) = rows %d types %v, want 0 [TEXT]", res.RowCount, res.ColumnTypes)
 		t.Fatalf("jsonb_array_elements_text(NULL) = rows %d types %v, want 0 [TEXT]", res.RowCount, res.ColumnTypes)
 	}
 	}
 }
 }
+
+// JSON1 $ path patch values may be non-integral numeric expressions. They are
+// normalized to json.Number before encoding, including values read from REAL
+// columns.
+func TestJSONBJSON1PatchFloatValues(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE realvals (id INTEGER PRIMARY KEY, r REAL)")
+	execMust(t, e, `INSERT INTO realvals VALUES (1, 1.5)`)
+
+	for _, tc := range []struct {
+		sql  string
+		want string
+	}{
+		{`SELECT jsonb_set('{"a":1}', '$.b', 1.5)`, `{"a":1,"b":1.5}`},
+		{`SELECT jsonb_set('{"a":1}', '$.b', 2.5)`, `{"a":1,"b":2.5}`},
+		{`SELECT jsonb_insert('{}', '$.b', 2.5)`, `{"b":2.5}`},
+		{`SELECT jsonb_replace('{"b":0}', '$.b', 2.5)`, `{"b":2.5}`},
+		{`SELECT jsonb_set('{"a":1}', '$.b', r) FROM realvals`, `{"a":1,"b":1.5}`},
+		{`SELECT jsonb_insert('{}', '$.b', r) FROM realvals`, `{"b":1.5}`},
+	} {
+		if got := jsonbValue(t, e, tc.sql); got != tc.want {
+			t.Errorf("%s = %v, want %s", tc.sql, got, tc.want)
+		}
+	}
+}
+
+// HAVING evaluates simple CASE and IS DISTINCT FROM through the aggregate
+// expression path; it must use the same exact jsonb comparison as the
+// top-level evaluators.
+func TestJSONBAggregateHavingExactJSONB(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE ex (id INTEGER PRIMARY KEY, doc JSONB)")
+	execMust(t, e, `INSERT INTO ex VALUES (1, '1'), (2, '1.0000000000000000001')`)
+
+	if got := jsonbValue(t, e, `SELECT COUNT(*) FROM ex GROUP BY doc HAVING '1'::jsonb IS DISTINCT FROM '1.0000000000000000001'::jsonb`); got != int64(1) {
+		t.Fatalf("HAVING IS DISTINCT FROM group count = %v, want 1", got)
+	}
+	res := execMust(t, e, `SELECT doc, COUNT(*) FROM ex GROUP BY doc HAVING '1'::jsonb IS DISTINCT FROM '1.0000000000000000001'::jsonb ORDER BY doc`)
+	if res.RowCount != 2 {
+		t.Fatalf("HAVING IS DISTINCT FROM rows = %v, want 2 groups", res.Rows)
+	}
+	res = execMust(t, e, `SELECT doc, COUNT(*) FROM ex GROUP BY doc HAVING '1'::jsonb IS NOT DISTINCT FROM '1.0000000000000000001'::jsonb`)
+	if res.RowCount != 0 {
+		t.Fatalf("HAVING IS NOT DISTINCT FROM rows = %v, want 0", res.Rows)
+	}
+	res = execMust(t, e, `SELECT doc, COUNT(*) FROM ex GROUP BY doc HAVING CASE '1'::jsonb WHEN '1.0000000000000000001'::jsonb THEN FALSE ELSE TRUE END ORDER BY doc`)
+	if res.RowCount != 2 {
+		t.Fatalf("HAVING CASE rows = %v, want 2 groups", res.Rows)
+	}
+}