Repository: geldata/gel
Pull request: Fix casts to scalars
Closes #5616

We've had a codepath to fix this bug for enum already, but now I extended it to anything that:
- is a scalar,
- it not the top-most concrete base of itself,
- is not an object (does not have a `ir_set.path_id`).


Problem statement:
casts from json to scalar subtypes don't check constraints
```
edgedb> create scalar type x extending str { create constraint expression on (__subject__ not like 'a%') };
OK: CREATE SCALAR TYPE
edgedb> select <x>to_json('"a"');
["a"]
edgedb> select <x><str>to_json('"a"');
edgedb error: ConstraintViolationError: invalid x
  Detail: invalid scalar type 'default::x'
```

` select <x>to_json('"a"')` ought to raise an error also.

(I don't think this is specific to json, of course.)
I think the issue is that we are only ever casting to `text`. We never emit a cast to the real target type.

Hints from issue discussion:

Commit message: fix

Patch to review:
diff --git a/edb/edgeql/compiler/casts.py b/edb/edgeql/compiler/casts.py
index 4c16b362308..11b9c825414 100644
--- a/edb/edgeql/compiler/casts.py
+++ b/edb/edgeql/compiler/casts.py
@@ -104,8 +104,6 @@ def compile_cast(
     ):
         return _find_object_by_id(ir_expr, new_stype, ctx=ctx)
 
-    json_t = ctx.env.get_schema_type_and_track(sn.QualName('std', 'json'))
-
     if isinstance(ir_set.expr, irast.Array):
         return _cast_array_literal(
             ir_set, orig_stype, new_stype, srcctx=srcctx, ctx=ctx)
@@ -145,6 +143,7 @@ def compile_cast(
             ir_set, orig_stype, new_stype,
             cardinality_mod=cardinality_mod, ctx=ctx)
 
+    json_t = ctx.env.get_schema_type_and_track(sn.QualName('std', 'json'))
     if (
         new_stype.issubclass(ctx.env.schema, json_t)
         and ir_set.path_id.is_objtype_path()
@@ -152,17 +151,21 @@ def compile_cast(
         # JSON casts of objects are special: we want the full shape
         # and not just an identity.
         viewgen.late_compile_view_shapes(ir_set, ctx=ctx)
-    else:
-        if orig_stype.issubclass(ctx.env.schema, json_t) and new_stype.is_enum(
-            ctx.env.schema
+    elif orig_stype.issubclass(ctx.env.schema, json_t):
+        if (
+            new_stype.is_enum(ctx.env.schema)
+            or (
+                isinstance(new_stype, s_scalars.ScalarType)
+                and not new_stype.get_builtin(ctx.env.schema)
+            )
         ):
-            # Casts from json to enums need some special handling
-            # here, where we have access to the enum type. Just turn
-            # it into json->str and str->enum.
+            # Casts from json to may have special handling.
+            # So we turn it into json->str and str->x.
             str_typ = ctx.env.get_schema_type_and_track(
                 sn.QualName('std', 'str')
             )
             str_ir = compile_cast(ir_expr, str_typ, srcctx=srcctx, ctx=ctx)
+
             return compile_cast(
                 str_ir,
                 new_stype,
@@ -171,12 +174,10 @@ def compile_cast(
                 ctx=ctx,
             )
 
-        if (
-            orig_stype.issubclass(ctx.env.schema, json_t)
-            and isinstance(new_stype, s_types.Array)
-            and not new_stype.get_subtypes(ctx.env.schema)[0].issubclass(
-                ctx.env.schema, json_t
-            )
+        elif isinstance(
+            new_stype, s_types.Array
+        ) and not new_stype.get_subtypes(ctx.env.schema)[0].issubclass(
+            ctx.env.schema, json_t
         ):
             # Turn casts from json->array<T> into json->array<json>
             # and array<json>->array<T>.
@@ -194,9 +195,7 @@ def compile_cast(
                 json_array_ir, new_stype, srcctx=srcctx, ctx=ctx
             )
 
-        if orig_stype.issubclass(ctx.env.schema, json_t) and isinstance(
-            new_stype, s_types.Tuple
-        ):
+        elif isinstance(new_stype, s_types.Tuple):
             return _cast_json_to_tuple(
                 ir_set,
                 orig_stype,
@@ -206,9 +205,7 @@ def compile_cast(
                 ctx=ctx,
             )
 
-        if orig_stype.issubclass(ctx.env.schema, json_t) and isinstance(
-            new_stype, s_types.Range
-        ):
+        elif isinstance(new_stype, s_types.Range):
             return _cast_json_to_range(
                 ir_set,
                 orig_stype,
diff --git a/tests/test_edgeql_casts.py b/tests/test_edgeql_casts.py
index c1b2e3b416c..889de7485f2 100644
--- a/tests/test_edgeql_casts.py
+++ b/tests/test_edgeql_casts.py
@@ -2624,6 +2624,25 @@ async def test_edgeql_casts_custom_scalar_05(self):
             [[42]],
         )
 
+    async def test_edgeql_casts_custom_scalar_06(self):
+        await self.con.execute(
+            '''
+            create scalar type x extending str {
+                create constraint expression on (false)
+            };
+        '''
+        )
+
+        async with self.assertRaisesRegexTx(
+            edgedb.ConstraintViolationError, 'invalid x'
+        ):
+            await self.con.query("""SELECT <x>42""")
+
+        async with self.assertRaisesRegexTx(
+            edgedb.ConstraintViolationError, 'invalid x'
+        ):
+            await self.con.query("""SELECT <x>to_json('"a"')""")
+
     async def test_edgeql_casts_tuple_params_01(self):
         # insert tuples into a nested array
         def nest(data):


Code review comments:
diff_hunk: @@ -145,24 +143,29 @@ def compile_cast(
             ir_set, orig_stype, new_stype,
             cardinality_mod=cardinality_mod, ctx=ctx)
 
+    json_t = ctx.env.get_schema_type_and_track(sn.QualName('std', 'json'))
     if (
         new_stype.issubclass(ctx.env.schema, json_t)
         and ir_set.path_id.is_objtype_path()
     ):
         # JSON casts of objects are special: we want the full shape
         # and not just an identity.
         viewgen.late_compile_view_shapes(ir_set, ctx=ctx)
-    else:
-        if orig_stype.issubclass(ctx.env.schema, json_t) and new_stype.is_enum(
-            ctx.env.schema
+    elif orig_stype.issubclass(ctx.env.schema, json_t):
+        if (
+            new_stype.is_enum(ctx.env.schema)
+            or (
+                isinstance(new_stype, s_scalars.ScalarType)
+                and not new_stype.get_builtin(ctx.env.schema)
+            )
         ):
-            # Casts from json to enums need some special handling
-            # here, where we have access to the enum type. Just turn
-            # it into json->str and str->enum.
+            # Casts from json to may have special handling.
+            # So we turn it into json->str and str->x.
path: edb/edgeql/compiler/casts.py
text: @user1:
We need to go through the scalar's base type, not always through str, I think