From c1a26b4703ba4131976416e0c40207ed5eb01b50 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 9 May 2024 18:18:22 -0400 Subject: [PATCH] Improve json cast error messages (#7312) Provide context to error messages when casting to from json to collections. Given `SELECT >to_json('{"b": 1}');` the following error is produced: ``` InvalidValueError: while casting 'std::json' to 'tuple', at tuple element 'a', missing value in JSON object ``` Given `SELECT >to_json('{"a": "b"}');` the following error is produced: ``` InvalidValueError: while casting 'std::json' to 'tuple', at tuple element 'a', expected JSON number or null; got JSON string ``` --- edb/buildmeta.py | 2 +- edb/edgeql/compiler/casts.py | 268 +++++++++++++++--------- edb/ir/ast.py | 1 + edb/lib/cal.edgeql | 19 +- edb/lib/cfg.edgeql | 2 +- edb/lib/std/30-jsonfuncs.edgeql | 259 ++++++++++++++--------- edb/pgsql/compiler/expr.py | 37 +++- edb/pgsql/delta.py | 11 +- edb/server/compiler/errormech.py | 16 +- tests/test_edgeql_casts.py | 340 +++++++++++++++++++++++++++++-- tests/test_edgeql_expressions.py | 24 +-- tests/test_edgeql_syntax.py | 2 +- 12 files changed, 740 insertions(+), 241 deletions(-) diff --git a/edb/buildmeta.py b/edb/buildmeta.py index fe6c128cf9a..be278b3b39a 100644 --- a/edb/buildmeta.py +++ b/edb/buildmeta.py @@ -55,7 +55,7 @@ # Increment this whenever the database layout or stdlib changes. -EDGEDB_CATALOG_VERSION = 2024_04_25_00_00 +EDGEDB_CATALOG_VERSION = 2024_05_09_00_00 EDGEDB_MAJOR_VERSION = 6 diff --git a/edb/edgeql/compiler/casts.py b/edb/edgeql/compiler/casts.py index 5d118b3a554..207a60b1182 100644 --- a/edb/edgeql/compiler/casts.py +++ b/edb/edgeql/compiler/casts.py @@ -22,6 +22,7 @@ from __future__ import annotations +import json from typing import ( Optional, Tuple, @@ -452,6 +453,7 @@ def _cast_to_ir( sql_function=cast.get_from_function(ctx.env.schema), sql_cast=cast.get_from_cast(ctx.env.schema), sql_expr=bool(cast.get_code(ctx.env.schema)), + error_message_context=cast_message_context(ctx), ) return setgen.ensure_set(cast_ir, ctx=ctx) @@ -477,6 +479,7 @@ def _inheritance_cast_to_ir( sql_function=None, sql_cast=True, sql_expr=False, + error_message_context=cast_message_context(ctx), ) return setgen.ensure_set(cast_ir, ctx=ctx) @@ -646,48 +649,72 @@ def _cast_json_to_tuple( ) -> irast.Set: with ctx.new() as subctx: + pathctx.register_set_in_scope(ir_set, ctx=subctx) subctx.anchors = subctx.anchors.copy() source_path = subctx.create_anchor(ir_set, 'a') # Top-level json->tuple casts should produce an empty set on - # null inputs, but error on missing fields or null - # subelements, so filter out json nulls directly here to - # distinguish those cases. - if cardinality_mod != qlast.CardinalityModifier.Required: - pathctx.register_set_in_scope(ir_set, ctx=subctx) - - check = qlast.FunctionCall( - func=('__std__', 'json_typeof'), args=[source_path] - ) - filtered = qlast.SelectQuery( - result=source_path, - where=qlast.BinOp( - left=check, - op='!=', - right=qlast.Constant.string('null'), - ) - ) - filtered_ir = dispatch.compile(filtered, ctx=subctx) - source_path = subctx.create_anchor(filtered_ir, 'a') + # null inputs, but error on missing fields or null subelements + allow_null = cardinality_mod != qlast.CardinalityModifier.Required + + # Only json arrays or objects can be cast to tuple. + # If not in the top level cast, raise an exception here + json_object_args: list[qlast.Expr] = [ + source_path, + qlast.Constant.boolean(allow_null), + ] + if error_message_context := cast_message_context(subctx): + json_object_args.append(qlast.Constant.string( + json.dumps({ + "error_message_context": error_message_context + }) + )) + json_objects = qlast.FunctionCall( + func=('__std__', '__tuple_validate_json'), + args=json_object_args + ) + + # Filter out json nulls. + # Nulls at the top level cast can be ignored. + filtered = qlast.SelectQuery( + result=json_objects, + where=qlast.BinOp( + left=qlast.FunctionCall( + func=('__std__', 'json_typeof'), args=[source_path] + ), + op='!=', + right=qlast.Constant.string('null'), + ), + ) + filtered_ir = dispatch.compile(filtered, ctx=subctx) + source_path = subctx.create_anchor(filtered_ir, 'a') # TODO: try using jsonb_to_record instead of a bunch of # json_get calls and see if that is faster. elements = [] for new_el_name, new_st in new_stype.iter_subtypes(ctx.env.schema): + cast_element = ('tuple', new_el_name) + if subctx.collection_cast_info is not None: + subctx.collection_cast_info.path_elements.append(cast_element) + + json_get_kwargs: dict[str, qlast.Expr] = {} + if error_message_context := cast_message_context(subctx): + json_get_kwargs['detail'] = qlast.Constant.string( + json.dumps({ + "error_message_context": error_message_context + }) + ) val_e = qlast.FunctionCall( - func=('__std__', 'json_get'), + func=('__std__', '__json_get_not_null'), args=[ source_path, qlast.Constant.string(new_el_name), ], + kwargs=json_get_kwargs ) val = dispatch.compile(val_e, ctx=subctx) - cast_element = ('tuple', new_el_name) - if subctx.collection_cast_info is not None: - subctx.collection_cast_info.path_elements.append(cast_element) - val = compile_cast( val, new_st, cardinality_mod=qlast.CardinalityModifier.Required, @@ -966,9 +993,19 @@ def _cast_json_to_range( with ctx.new() as subctx: subctx.anchors = subctx.anchors.copy() source_path = subctx.create_anchor(ir_set, 'a') + + check_args: list[qlast.Expr] = [source_path] + if error_message_context := cast_message_context(subctx): + check_args.append(qlast.Constant.string( + json.dumps({ + "error_message_context": error_message_context + }) + )) check = qlast.FunctionCall( - func=('__std__', '__range_validate_json'), args=[source_path] + func=('__std__', '__range_validate_json'), + args=check_args ) + check_ir = dispatch.compile(check, ctx=subctx) source_path = subctx.create_anchor(check_ir, 'b') @@ -977,30 +1014,107 @@ def _cast_json_to_range( bool_t = ctx.env.get_schema_type_and_track(sn.QualName('std', 'bool')) ql_bool_t = typegen.type_to_ql_typeref(bool_t, ctx=subctx) - cast = qlast.FunctionCall( - func=('__std__', 'range'), - args=[ - qlast.TypeCast( - expr=qlast.FunctionCall( - func=('__std__', 'json_get'), - args=[ - source_path, - qlast.Constant.string('lower'), - ], + def compile_with_range_element( + expr: qlast.Expr, + element_name: str, + ) -> irast.Set: + cast_element = ('range', element_name) + if subctx.collection_cast_info is not None: + subctx.collection_cast_info.path_elements.append(cast_element) + + expr_ir = dispatch.compile(expr, ctx=subctx) + + if subctx.collection_cast_info is not None: + subctx.collection_cast_info.path_elements.pop() + + return expr_ir + + lower: qlast.Expr = qlast.TypeCast( + expr=qlast.FunctionCall( + func=('__std__', 'json_get'), + args=[ + source_path, + qlast.Constant.string('lower'), + ], + ), + type=ql_range_el_t, + ) + lower_ir = compile_with_range_element(lower, 'lower') + lower = subctx.create_anchor(lower_ir, 'lower') + + upper: qlast.Expr = qlast.TypeCast( + expr=qlast.FunctionCall( + func=('__std__', 'json_get'), + args=[ + source_path, + qlast.Constant.string('upper'), + ], + ), + type=ql_range_el_t, + ) + upper_ir = compile_with_range_element(upper, 'upper') + upper = subctx.create_anchor(upper_ir, 'upper') + + inc_lower: qlast.Expr = qlast.TypeCast( + expr=qlast.FunctionCall( + func=('__std__', 'json_get'), + args=[ + source_path, + qlast.Constant.string('inc_lower'), + ], + kwargs={ + 'default': qlast.FunctionCall( + func=('__std__', 'to_json'), + args=[qlast.Constant.string("true")], ), - type=ql_range_el_t, - ), - qlast.TypeCast( - expr=qlast.FunctionCall( - func=('__std__', 'json_get'), - args=[ - source_path, - qlast.Constant.string('upper'), - ], + }, + ), + type=ql_bool_t, + ) + inc_lower_ir = compile_with_range_element(inc_lower, 'inc_lower') + inc_lower = subctx.create_anchor(inc_lower_ir, 'inc_lower') + + inc_upper: qlast.Expr = qlast.TypeCast( + expr=qlast.FunctionCall( + func=('__std__', 'json_get'), + args=[ + source_path, + qlast.Constant.string('inc_upper'), + ], + kwargs={ + 'default': qlast.FunctionCall( + func=('__std__', 'to_json'), + args=[qlast.Constant.string("false")], ), - type=ql_range_el_t, - ), - ], + }, + ), + type=ql_bool_t, + ) + inc_upper_ir = compile_with_range_element(inc_upper, 'inc_upper') + inc_upper = subctx.create_anchor(inc_upper_ir, 'inc_upper') + + empty: qlast.Expr = qlast.TypeCast( + expr=qlast.FunctionCall( + func=('__std__', 'json_get'), + args=[ + source_path, + qlast.Constant.string('empty'), + ], + kwargs={ + 'default': qlast.FunctionCall( + func=('__std__', 'to_json'), + args=[qlast.Constant.string("false")], + ), + }, + ), + type=ql_bool_t, + ) + empty_ir = compile_with_range_element(empty, 'empty') + empty = subctx.create_anchor(empty_ir, 'empty') + + cast = qlast.FunctionCall( + func=('__std__', 'range'), + args=[lower, upper], # inc_lower and inc_upper are required to be present for # non-empty casts from json, and this is checked in # __range_validate_json. We still need to provide default @@ -1008,54 +1122,9 @@ def _cast_json_to_range( # arguments to range are {} it will cause {"empty": true} # to evaluate to {}. kwargs={ - "inc_lower": qlast.TypeCast( - expr=qlast.FunctionCall( - func=('__std__', 'json_get'), - args=[ - source_path, - qlast.Constant.string('inc_lower'), - ], - kwargs={ - 'default': qlast.FunctionCall( - func=('__std__', 'to_json'), - args=[qlast.Constant.string("true")], - ), - }, - ), - type=ql_bool_t - ), - "inc_upper": qlast.TypeCast( - expr=qlast.FunctionCall( - func=('__std__', 'json_get'), - args=[ - source_path, - qlast.Constant.string('inc_upper'), - ], - kwargs={ - 'default': qlast.FunctionCall( - func=('__std__', 'to_json'), - args=[qlast.Constant.string("false")], - ), - }, - ), - type=ql_bool_t - ), - "empty": qlast.TypeCast( - expr=qlast.FunctionCall( - func=('__std__', 'json_get'), - args=[ - source_path, - qlast.Constant.string('empty'), - ], - kwargs={ - 'default': qlast.FunctionCall( - func=('__std__', 'to_json'), - args=[qlast.Constant.string("false")], - ), - }, - ), - type=ql_bool_t - ), + "inc_lower": inc_lower, + "inc_upper": inc_upper, + "empty": empty, } ) @@ -1273,6 +1342,7 @@ def _cast_array_literal( sql_cast=True, sql_expr=False, span=span, + error_message_context=cast_message_context(ctx), ) return setgen.ensure_set(cast_ir, ctx=ctx) @@ -1315,6 +1385,7 @@ def _cast_enum_str_immutable( sql_function=None, sql_cast=False, sql_expr=True, + error_message_context=cast_message_context(ctx), ) return setgen.ensure_set(cast_ir, ctx=ctx) @@ -1380,7 +1451,10 @@ def _find_object_by_id( def cast_message_context(ctx: context.ContextLevel) -> Optional[str]: - if ctx.collection_cast_info is not None: + if ( + ctx.collection_cast_info is not None + and ctx.collection_cast_info.path_elements + ): from_name = ( ctx.collection_cast_info.from_type.get_displayname(ctx.env.schema) ) @@ -1405,5 +1479,7 @@ def _collection_element_message_context( return f"at tuple element '{path_element[1]}', " elif path_element[0] == 'array': return f'in array elements, ' + elif path_element[0] == 'range': + return f"in range parameter '{path_element[1]}', " else: raise NotImplementedError diff --git a/edb/ir/ast.py b/edb/ir/ast.py index 4b67ff98275..d1095a772a2 100644 --- a/edb/ir/ast.py +++ b/edb/ir/ast.py @@ -1073,6 +1073,7 @@ class TypeCast(ImmutableExpr): sql_function: typing.Optional[str] = None sql_cast: bool sql_expr: bool + error_message_context: typing.Optional[str] = None @property def typeref(self) -> TypeRef: diff --git a/edb/lib/cal.edgeql b/edb/lib/cal.edgeql index ff06b936b34..96c0f2fd30c 100644 --- a/edb/lib/cal.edgeql +++ b/edb/lib/cal.edgeql @@ -1344,7 +1344,8 @@ CREATE CAST FROM std::json TO cal::local_datetime { SET volatility := 'Immutable'; USING SQL $$ SELECT edgedb.local_datetime_in( - edgedb.jsonb_extract_scalar(val, 'string')); + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) + ); $$; }; @@ -1352,7 +1353,9 @@ CREATE CAST FROM std::json TO cal::local_datetime { CREATE CAST FROM std::json TO cal::local_date { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.local_date_in(edgedb.jsonb_extract_scalar(val, 'string')); + SELECT edgedb.local_date_in( + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) + ); $$; }; @@ -1360,7 +1363,9 @@ CREATE CAST FROM std::json TO cal::local_date { CREATE CAST FROM std::json TO cal::local_time { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.local_time_in(edgedb.jsonb_extract_scalar(val, 'string')); + SELECT edgedb.local_time_in( + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) + ); $$; }; @@ -1368,7 +1373,9 @@ CREATE CAST FROM std::json TO cal::local_time { CREATE CAST FROM std::json TO cal::relative_duration { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'string')::interval::edgedb.relative_duration_t; + SELECT edgedb.jsonb_extract_scalar( + val, 'string', detail => detail + )::interval::edgedb.relative_duration_t; $$; }; @@ -1376,7 +1383,9 @@ CREATE CAST FROM std::json TO cal::relative_duration { CREATE CAST FROM std::json TO cal::date_duration { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.date_duration_in(edgedb.jsonb_extract_scalar(val, 'string')); + SELECT edgedb.date_duration_in( + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) + ); $$; }; diff --git a/edb/lib/cfg.edgeql b/edb/lib/cfg.edgeql index ba09b93b9bb..7292f2a5a2c 100644 --- a/edb/lib/cfg.edgeql +++ b/edb/lib/cfg.edgeql @@ -348,7 +348,7 @@ CREATE CAST FROM std::json TO cfg::memory { SET volatility := 'Immutable'; USING SQL $$ SELECT edgedb.str_to_cfg_memory( - edgedb.jsonb_extract_scalar(val, 'string') + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) ) $$; }; diff --git a/edb/lib/std/30-jsonfuncs.edgeql b/edb/lib/std/30-jsonfuncs.edgeql index 4c19cf0016e..5a64ccc2ff9 100644 --- a/edb/lib/std/30-jsonfuncs.edgeql +++ b/edb/lib/std/30-jsonfuncs.edgeql @@ -85,6 +85,31 @@ std::json_get( $$; }; + +CREATE FUNCTION +std::__json_get_not_null( + json: std::json, + VARIADIC path: std::str, + NAMED ONLY detail: std::str='') -> OPTIONAL std::json +{ + SET volatility := 'Immutable'; + SET internal := true; + USING SQL $$ + SELECT + CASE + WHEN "json" = 'null'::jsonb THEN + NULL + ELSE + edgedb.raise_on_null( + jsonb_extract_path("json", VARIADIC "path"), + 'invalid_parameter_value', + 'missing value in JSON object', + detail => detail + ) + END + $$; +}; + CREATE FUNCTION std::json_set( target: std::json, @@ -284,6 +309,36 @@ CREATE CAST FROM std::json TO anytuple { }; +CREATE FUNCTION +std::__tuple_validate_json( + v: std::json, + allow_null: std::bool, + detail: std::str='' + ) -> OPTIONAL std::json +{ + SET volatility := 'Immutable'; + SET internal := true; + USING SQL $$ + SELECT + CASE + WHEN v = 'null'::jsonb AND NOT allow_null THEN + edgedb.raise( + NULL::jsonb, + 'wrong_object_type', + msg => 'invalid null value in cast', + detail => detail + ) + ELSE + edgedb.jsonb_assert_type( + v, + ARRAY['array','object','null'], + detail => detail + ) + END; + $$; +}; + + CREATE CAST FROM std::json TO array { SET volatility := 'Immutable'; USING SQL $$ @@ -291,7 +346,9 @@ CREATE CAST FROM std::json TO array { CASE WHEN nullif(val, 'null'::jsonb) IS NULL THEN NULL ELSE (SELECT COALESCE(array_agg(j), ARRAY[]::jsonb[]) - FROM jsonb_array_elements(val) as j) + FROM jsonb_array_elements( + edgedb.jsonb_assert_type(val, ARRAY['array'], detail => detail) + ) as j) END ) $$; @@ -305,90 +362,103 @@ CREATE CAST FROM std::json TO array { CREATE FUNCTION -std::__range_validate_json(v: std::json) -> OPTIONAL std::json +std::__range_validate_json(val: std::json, detail: std::str='') -> OPTIONAL std::json { SET volatility := 'Immutable'; SET internal := true; USING SQL $$ - SELECT - CASE - WHEN v = 'null'::jsonb THEN - NULL - WHEN - empty - AND (lower IS DISTINCT FROM upper - OR lower IS NOT NULL AND inc_upper AND inc_lower) - THEN - edgedb.raise( - NULL::jsonb, - 'invalid_parameter_value', - msg => 'conflicting arguments in range constructor:' - || ' "empty" is `true` while the specified' - || ' bounds suggest otherwise' - ) + SELECT ( + SELECT + CASE + WHEN v = 'null'::jsonb THEN + NULL + WHEN + empty + AND (lower IS DISTINCT FROM upper + OR lower IS NOT NULL AND inc_upper AND inc_lower) + THEN + edgedb.raise( + NULL::jsonb, + 'invalid_parameter_value', + msg => 'conflicting arguments in range constructor:' + || ' ''empty'' is `true` while the specified' + || ' bounds suggest otherwise', + detail => detail + ) - WHEN - NOT empty - AND inc_lower IS NULL - THEN - edgedb.raise( - NULL::jsonb, - 'invalid_parameter_value', - msg => 'JSON object representing a range must include an' - || ' "inc_lower" boolean property' - ) + WHEN + NOT empty + AND inc_lower IS NULL + THEN + edgedb.raise( + NULL::jsonb, + 'invalid_parameter_value', + msg => 'JSON object representing a range must include an' + || ' ''inc_lower'' boolean property', + detail => detail + ) - WHEN - NOT empty - AND inc_upper IS NULL - THEN - edgedb.raise( - NULL::jsonb, - 'invalid_parameter_value', - msg => 'JSON object representing a range must include an' - || ' "inc_upper" boolean property' - ) + WHEN + NOT empty + AND inc_upper IS NULL + THEN + edgedb.raise( + NULL::jsonb, + 'invalid_parameter_value', + msg => 'JSON object representing a range must include an' + || ' ''inc_upper'' boolean property', + detail => detail + ) - WHEN - EXISTS ( - SELECT jsonb_object_keys(v) - EXCEPT - VALUES - ('lower'), - ('upper'), - ('inc_lower'), - ('inc_upper'), - ('empty') - ) - THEN - (SELECT edgedb.raise( - NULL::jsonb, - 'invalid_parameter_value', - msg => 'JSON object representing a range contains unexpected' - || ' keys: ' || string_agg(k.k, ', ' ORDER BY k.k) - ) - FROM - (SELECT jsonb_object_keys(v) - EXCEPT - VALUES - ('lower'), - ('upper'), - ('inc_lower'), - ('inc_upper'), - ('empty') - ) AS k(k) - ) - ELSE - v - END - FROM - (SELECT - (v ->> 'lower') AS lower, - (v ->> 'upper') AS upper, - (v ->> 'inc_lower')::bool AS inc_lower, - (v ->> 'inc_upper')::bool AS inc_upper, - coalesce((v ->> 'empty')::bool, false) AS empty - ) j + WHEN + EXISTS ( + SELECT jsonb_object_keys(v) + EXCEPT + VALUES + ('lower'), + ('upper'), + ('inc_lower'), + ('inc_upper'), + ('empty') + ) + THEN + (SELECT edgedb.raise( + NULL::jsonb, + 'invalid_parameter_value', + msg => 'JSON object representing a range contains unexpected' + || ' keys: ' || string_agg(k.k, ', ' ORDER BY k.k), + detail => detail + ) + FROM + (SELECT jsonb_object_keys(v) + EXCEPT + VALUES + ('lower'), + ('upper'), + ('inc_lower'), + ('inc_upper'), + ('empty') + ) AS k(k) + ) + ELSE + v + END + FROM + (SELECT + (v ->> 'lower') AS lower, + (v ->> 'upper') AS upper, + (v ->> 'inc_lower')::bool AS inc_lower, + (v ->> 'inc_upper')::bool AS inc_upper, + coalesce((v ->> 'empty')::bool, false) AS empty + ) j + ) + FROM ( + SELECT edgedb.jsonb_assert_type( + val, + ARRAY['object', 'null'], + detail => detail + ) AS v + ) $$; }; @@ -496,7 +566,7 @@ CREATE CAST FROM std::decimal TO std::json { CREATE CAST FROM std::json TO std::bool { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'boolean')::bool; + SELECT edgedb.jsonb_extract_scalar(val, 'boolean', detail => detail)::bool; $$; }; @@ -504,7 +574,7 @@ CREATE CAST FROM std::json TO std::bool { CREATE CAST FROM std::json TO std::uuid { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'string')::uuid; + SELECT edgedb.jsonb_extract_scalar(val, 'string', detail => detail)::uuid; $$; }; @@ -512,7 +582,10 @@ CREATE CAST FROM std::json TO std::uuid { CREATE CAST FROM std::json TO std::bytes { SET volatility := 'Immutable'; USING SQL $$ - SELECT decode(edgedb.jsonb_extract_scalar(val, 'string'), 'base64')::bytea; + SELECT decode( + edgedb.jsonb_extract_scalar(val, 'string', detail => detail), + 'base64' + )::bytea; $$; }; @@ -520,7 +593,7 @@ CREATE CAST FROM std::json TO std::bytes { CREATE CAST FROM std::json TO std::str { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'string'); + SELECT edgedb.jsonb_extract_scalar(val, 'string', detail => detail); $$; }; @@ -532,7 +605,9 @@ CREATE CAST FROM std::json TO std::datetime { # of the input string. SET volatility := 'Stable'; USING SQL $$ - SELECT edgedb.datetime_in(edgedb.jsonb_extract_scalar(val, 'string')); + SELECT edgedb.datetime_in( + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) + ); $$; }; @@ -540,7 +615,9 @@ CREATE CAST FROM std::json TO std::datetime { CREATE CAST FROM std::json TO std::duration { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.duration_in(edgedb.jsonb_extract_scalar(val, 'string')); + SELECT edgedb.duration_in( + edgedb.jsonb_extract_scalar(val, 'string', detail => detail) + ); $$; }; @@ -548,7 +625,7 @@ CREATE CAST FROM std::json TO std::duration { CREATE CAST FROM std::json TO std::int16 { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'number')::int2; + SELECT edgedb.jsonb_extract_scalar(val, 'number', detail => detail)::int2; $$; }; @@ -556,7 +633,7 @@ CREATE CAST FROM std::json TO std::int16 { CREATE CAST FROM std::json TO std::int32 { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'number')::int4; + SELECT edgedb.jsonb_extract_scalar(val, 'number', detail => detail)::int4; $$; }; @@ -564,7 +641,7 @@ CREATE CAST FROM std::json TO std::int32 { CREATE CAST FROM std::json TO std::int64 { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'number')::int8; + SELECT edgedb.jsonb_extract_scalar(val, 'number', detail => detail)::int8; $$; }; @@ -572,7 +649,7 @@ CREATE CAST FROM std::json TO std::int64 { CREATE CAST FROM std::json TO std::float32 { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'number')::float4; + SELECT edgedb.jsonb_extract_scalar(val, 'number', detail => detail)::float4; $$; }; @@ -580,7 +657,7 @@ CREATE CAST FROM std::json TO std::float32 { CREATE CAST FROM std::json TO std::float64 { SET volatility := 'Immutable'; USING SQL $$ - SELECT edgedb.jsonb_extract_scalar(val, 'number')::float8; + SELECT edgedb.jsonb_extract_scalar(val, 'number', detail => detail)::float8; $$; }; @@ -589,7 +666,7 @@ CREATE CAST FROM std::json TO std::decimal { SET volatility := 'Immutable'; USING SQL $$ SELECT edgedb.str_to_decimal( - edgedb.jsonb_extract_scalar(val, 'number') + edgedb.jsonb_extract_scalar(val, 'number', detail => detail) ); $$; }; @@ -599,7 +676,7 @@ CREATE CAST FROM std::json TO std::bigint { SET volatility := 'Immutable'; USING SQL $$ SELECT edgedb.str_to_bigint( - edgedb.jsonb_extract_scalar(val, 'number') + edgedb.jsonb_extract_scalar(val, 'number', detail => detail) ); $$; }; diff --git a/edb/pgsql/compiler/expr.py b/edb/pgsql/compiler/expr.py index 1e5950c060f..04f597202e9 100644 --- a/edb/pgsql/compiler/expr.py +++ b/edb/pgsql/compiler/expr.py @@ -202,6 +202,16 @@ def compile_TypeCast( pg_expr = dispatch.compile(expr.expr, ctx=ctx) + detail: Optional[pgast.StringConstant] = None + if expr.error_message_context is not None: + detail = pgast.StringConstant( + val=( + '{"error_message_context": "' + + expr.error_message_context + + '"}' + ) + ) + if expr.sql_cast: # Use explicit SQL cast. @@ -220,9 +230,13 @@ def compile_TypeCast( func_name = common.get_cast_backend_name( expr.cast_name, aspect="function" ) + + args = [pg_expr] + if detail is not None: + args.append(detail) res = pgast.FuncCall( name=func_name, - args=[pg_expr], + args=args, ) elif expr.sql_function: @@ -235,17 +249,20 @@ def compile_TypeCast( raise errors.UnsupportedFeatureError('cast not supported') if expr.cardinality_mod is qlast.CardinalityModifier.Required: + args = [ + res, + pgast.StringConstant( + val='invalid_parameter_value', + ), + pgast.StringConstant( + val='invalid null value in cast', + ), + ] + if detail is not None: + args.append(detail) res = pgast.FuncCall( name=('edgedb', 'raise_on_null'), - args=[ - res, - pgast.StringConstant( - val='invalid_parameter_value', - ), - pgast.StringConstant( - val='invalid null value in cast', - ), - ] + args=args ) return res diff --git a/edb/pgsql/delta.py b/edb/pgsql/delta.py index 578def9f7c9..6a531aa76c6 100644 --- a/edb/pgsql/delta.py +++ b/edb/pgsql/delta.py @@ -1932,10 +1932,13 @@ def make_cast_function(self, cast: s_casts.Cast, schema): name = common.get_backend_name( schema, cast, catenate=False, aspect='function') - args = [( - 'val', - types.pg_type_from_object(schema, cast.get_from_type(schema)) - )] + args: Sequence[dbops.FunctionArg] = [ + ( + 'val', + types.pg_type_from_object(schema, cast.get_from_type(schema)) + ), + ('detail', ('text',), "''"), + ] returns = types.pg_type_from_object(schema, cast.get_to_type(schema)) diff --git a/edb/server/compiler/errormech.py b/edb/server/compiler/errormech.py index 55dbd57597d..d03f5bcb3f6 100644 --- a/edb/server/compiler/errormech.py +++ b/edb/server/compiler/errormech.py @@ -453,9 +453,15 @@ def _static_interpret_invalid_param_value( err_details: ErrorDetails, from_graphql: bool = False, ): + error_message_context = '' + if err_details.detail_json: + error_message_context = ( + err_details.detail_json.get('error_message_context', '') + ) + return errors.InvalidValueError( - err_details.message, - details=err_details.detail if err_details.detail else None + error_message_context + err_details.message, + details=err_details.detail if err_details.detail else None, ) @@ -469,11 +475,15 @@ def _static_interpret_wrong_object_type( return SchemaRequired hint = None + error_message_context = '' if err_details.detail_json: hint = err_details.detail_json.get('hint') + error_message_context = ( + err_details.detail_json.get('error_message_context', '') + ) return errors.InvalidValueError( - err_details.message, + error_message_context + err_details.message, details=err_details.detail if err_details.detail else None, hint=hint, ) diff --git a/tests/test_edgeql_casts.py b/tests/test_edgeql_casts.py index 139404616dd..843f91774ea 100644 --- a/tests/test_edgeql_casts.py +++ b/tests/test_edgeql_casts.py @@ -2447,47 +2447,119 @@ async def test_edgeql_casts_json_11(self): [[1, 1, 2, 3, 5]] ) + # string to array async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'expected JSON number or null; got JSON string'): + r'expected JSON array; got JSON string'): + await self.con.query_single( + r"SELECT >'asdf'") + + # array of string to array of int + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'array', " + r"in array elements, " + r"expected JSON number or null; got JSON string"): await self.con.query_single( r"SELECT >['asdf']") async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'expected JSON number or null; got JSON string'): + r"while casting 'std::json' " + r"to 'array', " + r"in array elements, " + r"expected JSON number or null; got JSON string"): await self.con.query_single( r"SELECT >to_json('[1, 2, \"asdf\"]')") async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'std::json' " + r"to 'array', " + r"in array elements, " + r"expected JSON number or null; got JSON string"): + await self.con.execute(""" + SELECT >to_json('["a"]'); + """) + + # array with null to array + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'array' " + r"to 'array', " + r"in array elements, " + r"invalid null value in cast"): await self.con.query_single( r"SELECT >[to_json('1'), to_json('null')]") async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"array', " + r"in array elements, " + r"invalid null value in cast"): await self.con.query_single( r"SELECT >to_json('[1, 2, null]')") async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'array' " + r"to 'array', " + r"in array elements, " + r"invalid null value in cast"): await self.con.query_single( r"SELECT >>to_json('[1, 2, null]')") async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'std::json' " + r"to 'tuple>', " + r"at tuple element '0', " + r"invalid null value in cast"): await self.con.query_single( r"select >>to_json('[null]')") async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'cannot extract elements from a scalar'): + r"while casting 'std::json' " + r"to 'tuple>', " + r"at tuple element '0', " + r"in array elements, " + r"invalid null value in cast"): await self.con.query_single( - r"SELECT >'asdf'") + r"select >>to_json('[[null]]')") + + # object to array + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"expected JSON array; got JSON object"): + await self.con.execute(""" + SELECT >to_json('{"a": 1}'); + """) + + # array of object to array of scalar + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'array', " + r"in array elements, " + r"expected JSON number or null; got JSON object"): + await self.con.execute(""" + SELECT >to_json('[{"a": 1}]'); + """) + + # nested array + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'array>>', " + r"in array elements, " + r"at tuple element '0', " + r"in array elements, " + r"expected JSON string or null; got JSON number"): + await self.con.execute(""" + SELECT >>>to_json('[[[1]]]'); + """) async def test_edgeql_casts_json_12(self): self.assertEqual( @@ -2569,9 +2641,13 @@ async def test_edgeql_casts_json_12(self): edgedb.NamedTuple(a=12, b="bar")]) ) + # object with wrong element type to tuple async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'expected JSON number or null; got JSON string'): + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element 'b', " + r"expected JSON number or null; got JSON string"): await self.con.query( r""" SELECT > @@ -2579,24 +2655,46 @@ async def test_edgeql_casts_json_12(self): """ ) - # This isn't really the best error message for this. + # object with null value to tuple async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element 'a', " + r"invalid null value in cast"): + await self.con.query( + r"""SELECT >to_json('{"a": null}')""" + ) + + # object with missing element to tuple + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element 'b', " + r"missing value in JSON object"): await self.con.query( r"""SELECT >to_json('{"a": 1}')""" ) + # short array to unnamed tuple async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element '1', " + r"missing value in JSON object"): await self.con.query( r"""SELECT >to_json('[3000]')""" ) + # array to named tuple async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element 'a', " + r"missing value in JSON object"): await self.con.query( r""" SELECT > @@ -2604,20 +2702,90 @@ async def test_edgeql_casts_json_12(self): """ ) + # string to tuple async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r'expected JSON array or object or null; got JSON string'): await self.con.query( r"""SELECT > to_json('"test"')""" ) + # short array to unnamed tuple async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'invalid null value in cast'): + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element '1', " + r"missing value in JSON object"): await self.con.query( r"""SELECT > to_json('[3000]')""" ) + # object to unnamed tuple + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' to " + r"'tuple', " + r"at tuple element '0', " + r"missing value in JSON object"): + await self.con.execute(""" + SELECT >to_json('{"a": 1}'); + """) + + # nested tuple + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'tuple>', " + r"at tuple element 'a', " + r"at tuple element 'b', " + r"expected JSON string or null; got JSON number"): + await self.con.execute(""" + SELECT >>to_json('{"a": {"b": 1}}'); + """) + + # array with null to tuple + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element '1', " + r"invalid null value in cast"): + await self.con.execute(""" + SELECT >to_json('[1, null]'); + """) + + # object with null to tuple + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'tuple', " + r"at tuple element 'a', " + r"invalid null value in cast"): + await self.con.execute(""" + SELECT >to_json('{"a": null}'); + """) + + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'tuple>', " + r"at tuple element 'a', " + r"invalid null value in cast"): + await self.con.execute(""" + SELECT >>to_json('{"a": null}'); + """) + + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'tuple>', " + r"at tuple element 'a', " + r"invalid null value in cast"): + await self.con.execute(""" + SELECT >>to_json('{"a": null}'); + """) + async def test_edgeql_casts_json_13(self): await self.assert_query_result( r''' @@ -2708,6 +2876,148 @@ async def test_edgeql_casts_json_15(self): select Z union Z; ''') + async def test_edgeql_casts_json_16(self): + # number to range + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"expected JSON object or null; got JSON number"): + await self.con.execute(""" + SELECT >to_json('1'); + """) + + # array to range + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"expected JSON object or null; got JSON array"): + await self.con.execute(""" + SELECT >to_json('[1]'); + """) + + # object to range, bad empty + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'range', " + r"in range parameter 'empty', " + r"expected JSON boolean or null; got JSON number"): + await self.con.execute(""" + SELECT >to_json('{ + "empty": 1 + }'); + """) + + # object to range, empty with distinct lower and upper + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"conflicting arguments in range constructor: 'empty' is " + r"`true` while the specified bounds suggest otherwise"): + await self.con.execute(""" + SELECT >to_json('{ + "empty": true, + "lower": 1, + "upper": 2 + }'); + """) + + # object to range, empty with same lower and upper + # and inc_lower and inc_upper + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"conflicting arguments in range constructor: 'empty' is " + r"`true` while the specified bounds suggest otherwise"): + await self.con.execute(""" + SELECT >to_json('{ + "empty": true, + "lower": 1, + "upper": 2, + "inc_lower": true, + "inc_upper": true + }'); + """) + + # object to range, missing inc_lower + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"JSON object representing a range must include an " + r"'inc_lower'"): + await self.con.execute(""" + SELECT >to_json('{ + "inc_upper": false + }'); + """) + + # object to range, missing inc_upper + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"JSON object representing a range must include an " + r"'inc_upper'"): + await self.con.execute(""" + SELECT >to_json('{ + "inc_lower": false + }'); + """) + + # object to range, bad inc_lower + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'range', " + r"in range parameter 'inc_lower', " + r"expected JSON boolean or null; got JSON number"): + await self.con.execute(""" + SELECT >to_json('{ + "inc_lower": 1, + "inc_upper": false + }'); + """) + + # object to range, bad inc_upper + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"while casting 'std::json' " + r"to 'range', " + r"in range parameter 'inc_upper', " + r"expected JSON boolean or null; got JSON number"): + await self.con.execute(""" + SELECT >to_json('{ + "inc_lower": false, + "inc_upper": 1 + }'); + """) + + # object to range, extra parameters + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"JSON object representing a range contains unexpected keys: " + r"bar, foo"): + await self.con.execute(""" + SELECT >to_json('{ + "lower": 1, + "upper": 2, + "inc_lower": true, + "inc_upper": true, + "foo": "foo", + "bar": "bar" + }'); + """) + + async def test_edgeql_casts_json_17(self): + # number to multirange + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"expected JSON array; got JSON number"): + await self.con.execute(""" + SELECT >to_json('1'); + """) + + # object to multirange + async with self.assertRaisesRegexTx( + edgedb.InvalidValueError, + r"expected JSON array; got JSON object"): + await self.con.execute(""" + SELECT >to_json('{"a": 1}'); + """) + async def test_edgeql_casts_assignment_01(self): async with self._run_and_rollback(): await self.con.execute(r""" diff --git a/tests/test_edgeql_expressions.py b/tests/test_edgeql_expressions.py index 5653614ddaa..346c014c54d 100644 --- a/tests/test_edgeql_expressions.py +++ b/tests/test_edgeql_expressions.py @@ -6796,8 +6796,8 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_upper"' - r' boolean property' + r"JSON object representing a range must include an 'inc_upper'" + r" boolean property" ): await self.con.execute(r""" select >to_json('{ @@ -6810,8 +6810,8 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_lower"' - r' boolean property' + r"JSON object representing a range must include an 'inc_lower'" + r" boolean property" ): await self.con.execute(r""" select >to_json('{ @@ -6823,8 +6823,7 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_lower"' - r' boolean property' + r"expected JSON object or null; got JSON array" ): await self.con.execute(r""" select >to_json('["bad", null]'); @@ -6832,8 +6831,8 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_lower"' - r' boolean property' + r"JSON object representing a range must include an 'inc_lower'" + r" boolean property" ): await self.con.execute(r""" select >to_json('{"bad": null}'); @@ -6841,8 +6840,7 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_lower"' - r' boolean property' + r"expected JSON object or null; got JSON string" ): await self.con.execute(r""" select >to_json('"bad"'); @@ -6850,8 +6848,7 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_lower"' - r' boolean property' + r"expected JSON object or null; got JSON number" ): await self.con.execute(r""" select >to_json('1312'); @@ -6859,8 +6856,7 @@ async def test_edgeql_expr_range_34(self): async with self.assertRaisesRegexTx( edgedb.InvalidValueError, - r'JSON object representing a range must include an "inc_lower"' - r' boolean property' + r"expected JSON object or null; got JSON boolean" ): await self.con.execute(r""" select >to_json('true'); diff --git a/tests/test_edgeql_syntax.py b/tests/test_edgeql_syntax.py index 210aa2c063d..b3fb2357dbd 100644 --- a/tests/test_edgeql_syntax.py +++ b/tests/test_edgeql_syntax.py @@ -5266,7 +5266,7 @@ def test_edgeql_syntax_ddl_cast_03(self): SET volatility := 'Stable'; USING SQL $$ SELECT edgedb.str_to_bigint( - edgedb.jsonb_extract_scalar(val, 'number') + edgedb.jsonb_extract_scalar(val, 'number', detail => detail) ); $$; };