edgedb-cli icon indicating copy to clipboard operation
edgedb-cli copied to clipboard

Cast expression suggestion when changing type of a link property references it as a standard property

Open raddevon opened this issue 1 year ago • 3 comments

When altering the type of a link property as shown in the schema at the bottom:

did you alter the type of property 'date' of link 'habit_completed'? [y,n,l,c,b,s,q,?]
> y
Please specify a conversion expression to alter the type of property 'date':
cast_expr> <cal::local_date>.date

Suggestion is to cast a property .date which does not exist on the type. The suggestion should be <cal::local_date>@date instead.

  • EdgeDB Version: 5.0-dev.8091+e24f7fd
  • EdgeDB CLI Version: 4.0.2+500be79
  • OS Version: macOS 14.0

Steps to Reproduce

  1. Create a schema with a link property
  2. Migrate
  3. Change the type of the link property
  4. Migrate again

Schema

Start:

module default {
    type Habit {
      required name: str;
    }
    type User {
      name: str;
      multi habit_completed: Habit {
        date: datetime;
        # Ensures a User can complete a given Habit
        # only once per day.
        constraint exclusive on ((@target, @date));
      }
    }
}

End:

module default {
    type Habit {
      required name: str;
    }
    type User {
      name: str;
      multi habit_completed: Habit {
        date: cal::local_date;
        # Ensures a User can complete a given Habit
        # only once per day.
        constraint exclusive on ((@target, @date));
      }
    }
}

raddevon avatar Dec 12 '23 14:12 raddevon

Interesting issue, noticed a few things:

  • The original schema doesn't work but does if you constrain on just @date (and reproduces the error)
  • I think this is an edgedb and not edgedb-cli issue because the CLI just uses create migration to (schema) and then describe current migration as json which gives it this prompt from the server which it simply follows:
  "parent": "m1t2it77gg4vsiznipiim453v3b7z6hhjy3v6j7e2xumflsmwjtc7q",
  "complete": false,
  "proposed": {
    "prompt": "did you alter the type of property 'date' of link 'habit_completed'?",
    "data_safe": false,
    "prompt_id": "SetPropertyType PROPERTY default::__|date@default|__||habit_completed&default||User",
    "confidence": 1.0,
    "statements": [
      {
        "text": "ALTER TYPE default::User {\n    ALTER LINK habit_completed {\n        ALTER PROPERTY date {\n            SET TYPE cal::local_date USING (\\(cast_expr));\n        };\n    };\n};"
      }
    ],
    "required_user_input": [
      {
        "prompt": "Please specify a conversion expression to alter the type of property 'date'",
        "new_type": "cal::local_date",
        "old_type": "std::datetime",
        "placeholder": "cast_expr",
        "pointer_name": "date",
        "new_type_is_object": false,
        "old_type_is_object": false
      }
    ]
  },
  "confirmed": []
}

(Correction: looks like this is indeed a CLI issue: per @msullivan the server just reports the old and new type) https://github.com/edgedb/edgedb-cli/blob/master/src/migrations/create.rs#L332

The suggested cast is produced by the CLI, not by the compiler/server. The server just reports the old_type and the new_type back to the CLI

  • Casting into a cal::local_date won't work in any case whereas something like this will: cal::to_local_date(@date, 'UTC') So maybe the prompt should be cal::to_local_date(@date, <timezone>) or something like that? Since the local_date will vary depending on which timezone is being used as the local time.

Dhghomon avatar Dec 13 '23 00:12 Dhghomon

You're correct that the original cast suggestion won't work, even if it references the property correctly, but I wasn't sure how sophisticated and bespoke those suggestions could be. Yeah, this suggestion you made (cal::to_local_date(@date, <timezone>)) is basically what I ended up using to do the cast.

So, maybe this is actually two issues:

  1. reference link properties properly when giving a cast expression suggestion and
  2. give a working suggestion when changing a datetime property to a cal::local_date

raddevon avatar Dec 13 '23 12:12 raddevon

You're correct that the original cast suggestion won't work, even if it references the property correctly, but I wasn't sure how sophisticated and bespoke those suggestions could be. Yeah, this suggestion you made (cal::to_local_date(@date, <timezone>)) is basically what I ended up using to do the cast.

So, maybe this is actually two issues:

  1. reference link properties properly when giving a cast expression suggestion and
  2. give a working suggestion when changing a datetime property to a cal::local_date

I was looking into this today and looks like it's pretty easy to just query the database with select schema::Cast and select schema::Function and then put together a HashMap to tell which casts are doable, and if not castable then which functions could work. So I think I can make this more intuitive in addition to the link property fix.

Dhghomon avatar Dec 13 '23 13:12 Dhghomon