More intelligent migrations

[Dhghomon]

Resolves #1188 as well as adds two more improvements: checking if a cast will even work in the first place, and looking for any helpful functions to do the job if not.

• tl;dr: Can prompt_id be relied on or might it change in the future?

Current behaviour: when a property is changed to a new scalar type, the CLI will simply suggest a cast from the old type to the new type. This will often work but not every scalar type can be cast into another scalar type, plus it doesn't recognize when a property is a link property and requires @ instead of . (which is what 1188 above is about).

New behaviour:

• First check to see if a type can even be cast into the new type. If so, check if there are link properties and regular properties with the same name and output a message accordingly. Gives a . if the property is a regular property, @ if it's a link property, and . with a note to the user to change to @ if the property is from a link.
• If not, just return the pointer using the same logic as above, but first checking to see if there are any functions that fit the bill. Fit the bill = single parameter and single return type that match the old and new type.

Now here's the part I'm looking for feedback on:

Right now the old-to-new pointer logic is done inside a function that only has access to required_user_input, which only has the following:

// "required_user_input": [
// {
// "prompt": "Please specify a conversion expression to alter the type of property 'strength'",
// "new_type": "std::str",
// "old_type": "std::int64",
// "placeholder": "cast_expr",
// "pointer_name": "strength",
// "new_type_is_object": false,
// "old_type_is_object": false
// }

So with that you can do a query on whether the type name matches regular or link properties somewhere inside the schema, but not necessarily for the current type being modified. That means that if there is a duplicate name anywhere in the schema the user will still get the output asking them to make a choice between . and @.

This could be improved by passing on the entire Proposal output, which looks like this:

// Example:
//
// "proposed": {
// "prompt": "did you alter the type of property 'strength' of link 'times'?",
// "data_safe": false,
// "prompt_id": "SetPropertyType PROPERTY default::|strength@default|||times&default||Time",
// "confidence": 1.0,
// "statements": [
// {
// "text": "ALTER TYPE default::Time {\n ALTER LINK times {\n ALTER PROPERTY strength {\n SET TYPE std::str USING (\(cast_expr));\n };\n };\n};"
// }

I'm curious how set in stone the output for prompt_id is. If it is then it could be used, but if future output from the server might change then best not to pass it on.

Schema to try out changes:

module default {
    type SharesAName {
        # Change one or both to another scalar type, should advise user to change . to @ if is a link property
        multi some_link: SharesAName {
            shares_a_name: int64;
        }
        shares_a_name: int64;
    }

    type DoesNotShareName {
        multi other_link: DoesNotShareName {
            # Change this to another scalar type, should generate cast with @
            does_not_share_name: int64;
        }
    }

    type TimeType {
        # Change to datetime, should suggest to_datetime function
        int_to_datetime: int64;
    }

    type StrToArrayStr {
        # Change to array<str>, should suggest a number of functions including
        # the user-defined one below
        s: str;
    }

    function str_to_array_str(input: str) -> array<str> using (
        [input]
    );
}

[raddevon]

Love what you're doing with these, @Dhghomon. A few things I wanted to ask about:

Note: The following functions may help you convert from array<std::str> to std::int64:
  std::len(array<anytype>) -> std::int64
  std::count(anytype) -> std::int64
  std::enumerate(anytype) -> tuple<std::int64, anytype>
Please specify a conversion expression to alter the type of property 'b':
cast_expr_1> @b

In this instance, count and enumerate don't feel particularly useful since they will both operate on sets. len could make sense depending on the use case. For example, if I have an array in one of my objects that is [1,2,3], passing that to len gives me 3 while passing to count gives me 1 and passing to enumerate gives me (0, [1, 2, 3]). Hard to predict if len would be useful either, but it does take the input type and return the output type. The others do technically, but only because everything is a set. Suggesting functions is a really cool trick, but I wonder how useful it is when the match is based on anytype. Maybe those matches go in a separate category of "less likely but also possibly useful" functions?

Not sure if a cast using enumerate here is useful since that will return a tuple for an int64 field. I guess it was matched because one of the elements of the tuple is a matching type?

Note unrelated to this example: as I'm going through the migration questionnaire, it occurs to me it would be really great to see a few lines of context from the SDL as I'm being asked the questions. Instead, I'm flipping back and forth between tabs so I can cross-reference what it's asking me about.

did you alter the type of property 'has_same_name' of link 'another_link'? [y,n,l,c,b,s,q,?]
> y
Note: The following functions may help you convert from array<std::str> to std::int64:
  std::len(array<anytype>) -> std::int64
  std::count(anytype) -> std::int64
  std::enumerate(anytype) -> tuple<std::int64, anytype>
Note: Your schema has both object and link properties with the name `has_same_name`.
If this object has both, then:
 .has_same_name will access the object type property, while
 @has_same_name will access the link property.
Please specify a conversion expression to alter the type of property 'has_same_name':
cast_expr> has_same_name

Same feedback as the previous question regarding array vs. set functions. The suggestion here doesn't work without user modification. I think that's intentional. If it is, I might prefer leaving it blank to avoid suggesting we've provided something that may just work. (This could be feedback on the previous question as well now that I think about it. Maybe we need to consider how we feel more generally about providing a default cast expression that requires modification versus giving the user a blank field.)

If we want to suggest something here, in this particular context, I think it makes sense to make the default value the property in question. Here's what the theoretical alternative might look like:

did you alter the type of property 'has_same_name' of link 'another_link'? [y,n,l,c,b,s,q,?]
> y
Note: The following functions may help you convert from array<std::str> to std::int64:
  std::len(array<anytype>) -> std::int64
  std::count(anytype) -> std::int64
  std::enumerate(anytype) -> tuple<std::int64, anytype>
Note: Your schema has both object and link properties with the name `has_same_name`.
If this object has both, then:
 @has_same_name will access the link property (this property), while
 .has_same_name will access the object type property.
Please specify a conversion expression to alter the type of property 'has_same_name':
cast_expr> @has_same_name

Changes here:

• Flip the order of the properties to present the current one (the link property) first
• Call out that the link property is the one we're asking about (via (this property))
• Set the value of the cast_expr field to the current property: @has_same_name

My assumption here is that, if you're migrating a given property, its new value is more likely to be based on its current value than on the value of a different, similarly named property.

It might also be good to make sure we refer to it as a "link property" in the question. I know we immediately follow "property" with "of link," but even that's long enough for my brain to disconnect and lose the context.

did you alter the type of link property 'has_same_name' of link 'another_link'? [y,n,l,c,b,s,q,?]

just to be 100% explicit about what the user is dealing with here.

did you alter the type of property 'does_not_share_name' of link 'other_link'? [y,n,l,c,b,s,q,?]
> y

This one didn't suggest a cast at all. It moved right on to the next question. 🤔 Not sure why or what the implications of that are going to be.

did you alter object type 'default::SharesAName'? [y,n,l,c,b,s,q,?]
> y
Note: Your schema has both object and link properties with the name `shares_a_name`.
If this object has both, then:
 <std::str>.shares_a_name will cast from the object type property, while
 <std::str>@shares_a_name will cast from the link property.
Please specify a conversion expression to alter the type of property 'shares_a_name':
cast_expr> <std::str>_shares_a_name

The way this one suggests the output against the ambiguity is different from the first example. In the first example, we fill the cast_expr field with has_same_name since we don't know if it should be prefaced with . or @. In this case, though, we preface it with _. I think either might work, but we want to do it consistently. Again, what we should do here should come back to how we want to treat a scenario where we can't guess what should be done with much confidence. Maybe the right answer is to provide some hints but to suggest nothing in the field.

The more problematic issue with this one is that I changed both the property and the link property, but the output here has not indicated which it's asking me about. I can work backwards and, since I changed one to str and the other to a different type, figure out that this probably references that one based on the hints. (Not sure how it would have behaved if I had changed both to the same type.) I think it should probably be more explicit about this though. It feels like it's missing one question before the casting.

I went forward and was surprised that it didn't ask about the other change, which was the shares_a_name property from int64 to int16. Maybe this is because I went from integer type to integer type, but it seems like I should still get an opportunity to write a cast expression. I may just be confused about this.

did you alter the type of property 's' of object type 'default::StrToArrayStr'? [y,n,l,c,b,s,q,?]
> y
Note: The following functions may help you convert from std::str to array<std::str>:
  std::re_match(std::str) -> array<std::str>
  std::re_match_all(std::str) -> array<std::str>
  std::str_split(std::str) -> array<std::str>
  default::str_to_array_str(std::str) -> array<std::str>
Please specify a conversion expression to alter the type of property 's':
cast_expr> .s

Very cool that the user-defined function is suggested! That feels kinda like magic.

[Dhghomon]

Love what you're doing with these, @Dhghomon. A few things I wanted to ask about:

Note: The following functions may help you convert from array<std::str> to std::int64:
  std::len(array<anytype>) -> std::int64
  std::count(anytype) -> std::int64
  std::enumerate(anytype) -> tuple<std::int64, anytype>
Please specify a conversion expression to alter the type of property 'b':
cast_expr_1> @b

In this instance, count and enumerate don't feel particularly useful since they will both operate on sets. len could make sense depending on the use case. For example, if I have an array in one of my objects that is [1,2,3], passing that to len gives me 3 while passing to count gives me 1 and passing to enumerate gives me (0, [1, 2, 3]). Hard to predict if len would be useful either, but it does take the input type and return the output type. The others do technically, but only because everything is a set. Suggesting functions is a really cool trick, but I wonder how useful it is when the match is based on anytype. Maybe those matches go in a separate category of "less likely but also possibly useful" functions?

Not sure if a cast using enumerate here is useful since that will return a tuple for an int64 field. I guess it was matched because one of the elements of the tuple is a matching type?

True, looking at the functions that have anytype, anyreal etc. they aren't that useful when it comes down to it. I've slimmed the check down to something simpler that just looks for complete name equality for the input and return types.

Note unrelated to this example: as I'm going through the migration questionnaire, it occurs to me it would be really great to see a few lines of context from the SDL as I'm being asked the questions. Instead, I'm flipping back and forth between tabs so I can cross-reference what it's asking me about.

did you alter the type of property 'has_same_name' of link 'another_link'? [y,n,l,c,b,s,q,?]
> y
Note: The following functions may help you convert from array<std::str> to std::int64:
  std::len(array<anytype>) -> std::int64
  std::count(anytype) -> std::int64
  std::enumerate(anytype) -> tuple<std::int64, anytype>
Note: Your schema has both object and link properties with the name `has_same_name`.
If this object has both, then:
 .has_same_name will access the object type property, while
 @has_same_name will access the link property.
Please specify a conversion expression to alter the type of property 'has_same_name':
cast_expr> has_same_name

Same feedback as the previous question regarding array vs. set functions. The suggestion here doesn't work without user modification. I think that's intentional. If it is, I might prefer leaving it blank to avoid suggesting we've provided something that may just work. (This could be feedback on the previous question as well now that I think about it. Maybe we need to consider how we feel more generally about providing a default cast expression that requires modification versus giving the user a blank field.)

Yeah. Maybe the best output when the name of a property is both that of a regular property and a link property is to just default to . but with a note that the user should change it to @ if it refers to a link property. I've changed to that.

If we want to suggest something here, in this particular context, I think it makes sense to make the default value the property in question. Here's what the theoretical alternative might look like:

did you alter the type of property 'has_same_name' of link 'another_link'? [y,n,l,c,b,s,q,?]
> y
Note: The following functions may help you convert from array<std::str> to std::int64:
  std::len(array<anytype>) -> std::int64
  std::count(anytype) -> std::int64
  std::enumerate(anytype) -> tuple<std::int64, anytype>
Note: Your schema has both object and link properties with the name `has_same_name`.
If this object has both, then:
 @has_same_name will access the link property (this property), while
 .has_same_name will access the object type property.
Please specify a conversion expression to alter the type of property 'has_same_name':
cast_expr> @has_same_name

Changes here:

• Flip the order of the properties to present the current one (the link property) first
• Call out that the link property is the one we're asking about (via (this property))
• Set the value of the cast_expr field to the current property: @has_same_name

My assumption here is that, if you're migrating a given property, its new value is more likely to be based on its current value than on the value of a different, similarly named property.

It might also be good to make sure we refer to it as a "link property" in the question. I know we immediately follow "property" with "of link," but even that's long enough for my brain to disconnect and lose the context.

did you alter the type of link property 'has_same_name' of link 'another_link'? [y,n,l,c,b,s,q,?]

just to be 100% explicit about what the user is dealing with here.

did you alter the type of property 'does_not_share_name' of link 'other_link'? [y,n,l,c,b,s,q,?]
> y

This output comes directly from the compiler on the non-Rust side so maybe it could be changed there in the future. I agree that it would be nice to be a tad more explicit.

Very cool that the user-defined function is suggested! That feels kinda like magic.

Yeah! Was interesting to poke through queries on schema::Function for this and see that the user-defined ones show up in the exact same way as the rest.