Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to inherit enum type #3578

Closed
haphamdev opened this issue Mar 5, 2022 · 2 comments · Fixed by #3593
Closed

Unable to inherit enum type #3578

haphamdev opened this issue Mar 5, 2022 · 2 comments · Fixed by #3593
Assignees

Comments

@haphamdev
Copy link

  • EdgeDB Version: EdgeDB CLI 1.1.0+8bc2c20
  • Server version: 1.1
  • OS Version: Mac OS Catalina 10.15.7

Steps to Reproduce:

  1. Just create a new instance with following schema
    scalar type Status extending enum<pending, in_progress, finished>;
    scalar type ImportStatus extending Status;
    scalar type ImportAnalyticsStatus extending Status;
  1. Running edgedb migration create and get error message
> edgedb migration create
did you create scalar type 'default::Status'? [y,n,l,c,b,s,q,?]
> y
did you create scalar type 'default::ImportAnalyticsStatus'? [y,n,l,c,b,s,q,?]
> y
did you create scalar type 'default::ImportStatus'? [y,n,l,c,b,s,q,?]
> y
edgedb error: EdgeDB could not resolve migration with the provided answers. Please retry with different answers.

@msullivan
Copy link
Member

Yeah it looks like this is just not really implemented. I'm going to put up a PR to produce a real error message, for now.

msullivan added a commit that referenced this issue Mar 9, 2022
The main thing is that the subtypes ought to be domain types, not enum
types themselves.

THe bulk of the code, however, all has to deal with fixing the
_undo_everything/_redo_everything scheme in the presence of inherited
enum types. The inherited enums need to be able to be deleted
temporarily if an element is being dropped from the parent type, and
that needs to be properly ordered with respect to properties that
depend on it. We deal with this by pulling the core part of the
property type switcharoos into the main sorted loop instead of having
it be separate.

Fixes #3578.
@msullivan
Copy link
Member

Alright, I actually ended up taking a second look and realized that actually implementing it would be pretty easy.

And then, after I had finished all the parts that I felt were the core of the implementation, I stumbled across some very fiddly interactions with our mechanism for deleting an element from an enum, which wound up requiring some nontrivial changes to that mechanism.

msullivan added a commit that referenced this issue Mar 9, 2022
The main thing is that the subtypes ought to be domain types, not enum
types themselves.

THe bulk of the code, however, all has to deal with fixing the
_undo_everything/_redo_everything scheme in the presence of inherited
enum types. The inherited enums need to be able to be deleted
temporarily if an element is being dropped from the parent type, and
that needs to be properly ordered with respect to properties that
depend on it. We deal with this by pulling the core part of the
property type switcharoos into the main sorted loop instead of having
it be separate.

Fixes #3578.
msullivan added a commit that referenced this issue Mar 9, 2022
The main thing is that the subtypes ought to be domain types, not enum
types themselves.

THe bulk of the code, however, all has to deal with fixing the
_undo_everything/_redo_everything scheme in the presence of inherited
enum types. The inherited enums need to be able to be deleted
temporarily if an element is being dropped from the parent type, and
that needs to be properly ordered with respect to properties that
depend on it. We deal with this by pulling the core part of the
property type switcharoos into the main sorted loop instead of having
it be separate.

Fixes #3578.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants