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 use UNLESS CONFLICT with user assigned .id #4302

Open
sonnes opened this issue Aug 27, 2022 · 14 comments
Open

Unable to use UNLESS CONFLICT with user assigned .id #4302

sonnes opened this issue Aug 27, 2022 · 14 comments
Assignees
Labels

Comments

@sonnes
Copy link

sonnes commented Aug 27, 2022

i am trying to create a copy of existing data on edgedb. all my entities already use UUIDs, so I am trying to upsert using

with
    id := <uuid>$id,
    display_name := <str>$display_name
insert Brand {
    id := id,
    display_name := display_name
}
unless conflict on (.id)
else (
    update Brand
    filter .id = id
    set {
        display_name := display_name
    }
);

but i see that id cannot be used with else clause. i see this error UNLESS CONFLICT can not use ELSE when constraint is from a parent type

  • EdgeDB Version: 2.1+52c90a7

Schema:

module default {
    type Brand {
        required property display_name -> str;
        required property tenant -> str;
        multi link outlets -> Outlet;
    }

    type Outlet {
        required property tenant -> str;
        required property display_name -> str;
        link brand -> Brand;
        multi link items -> Item;
    }

    type Item {
        required property tenant -> str;
        required property display_name -> str;
        link outlet -> Outlet;
    }
}
@sonnes
Copy link
Author

sonnes commented Aug 27, 2022

@msullivan
Copy link
Member

The tricky thing in this case is that the id that conflicts might be from a different type, and there are semantic questions about what should happen in an else branch in this case.
Do we execute the else branch? What names do we bind in it?
It's definitely implementable, but it's likely to be somewhat confusing. Maybe there is space for more syntax to clarify the situation, but that has downsides too.

Maybe we want a way to say "unless conflict if it conflicts with something of the same type, otherwise error"

@elprans
Copy link
Member

elprans commented Sep 20, 2022

One way would be to bind something like a __conflict__ magic to the origin type of the constraint and prohibit binding the insert subject type in those cases. A bit ugly, but unambiguous.

In addition we can make the "unless conflict if it conflicts with something of the same type, otherwise error" to be the default behavior (with a clear explanation in the raised ConstraintViolationError as to why UNLESS CONFLICT didn't catch it).

@elprans
Copy link
Member

elprans commented Sep 20, 2022

(also, we really need to be including the "detail" and "hint" in client binding error renderings)

@msullivan
Copy link
Member

The nice thing, though, is that else is typically not needed, it is just nice to have.
So here, you can do

with
    id := <uuid>$id,
    display_name := <str>$display_name,
    ins := (
      insert Brand { id := id, display_name := display_name }
      unless conflict on (.id)
    ),
    uid := (select id filter id not in ins.id),
    upd := (
      update Brand filter .id = uid
      set { display_name := display_name }
    ),
select {ins, upd};

@msullivan msullivan added the under discussion Not ready for implementation label Sep 20, 2022
@msullivan msullivan removed their assignment Sep 20, 2022
@AnsonHwang86
Copy link
Contributor

any update? thanks

@sonnes
Copy link
Author

sonnes commented Jan 2, 2023

Apologies for the late response. This is not an issue for me anymore. The solution from @msullivan works fine.

@sirkonst
Copy link

sirkonst commented Apr 3, 2023

Faced with it too, really annoying because it's not an intuitive and workaround looks very ugly :-(

@msullivan msullivan self-assigned this Jul 26, 2023
@TomLisankie
Copy link

Maybe we want a way to say "unless conflict if it conflicts with something of the same type, otherwise error"

This seems like the ideal default behavior.

@AnsonHwang86
Copy link
Contributor

image
It seem a bug. If I change * to ** it will work.

@AnsonHwang86
Copy link
Contributor

image

when I run the above edgeql, the return value don't the latest one.
When I checked the database, it indeed updated.

image

@AnsonHwang86
Copy link
Contributor

Hello, could this feature be added to v4? I am waiting to construct a perfect batch upsert( The way I currently implement it is to use a multi step with help of js) in pure edgeql. Considering the conditional DML will support in v4, if I can use this with it, that would save life.

@msullivan
Copy link
Member

This won't be changed in 4.0, but conditional DML gives another way to handle it, by doing (update ...) ?? (insert ...).

@AnsonHwang86
Copy link
Contributor

I just test conditional DML in 4.0, it perfect solve this problem. So strong,This problem no longer bothers me anymore.

Here is my solution.

with users := <json>$user,
updateds:=(
  for item in json_array_unpack(users) union( 
  (update User filter .id=<uuid>item["id"] set {name:=<str>item["name"]})??
  (insert User{name:=<str>item["name"]}))
  ), 
select updateds{*};
[{"id":"a0ae3356-730f-11ee-bd86-cf0231420442","name":"aaaaa"},{"id":"a62a35c8-730f-11ee-95b6-d3c0e5fa3803","name":"cccc"}]

image

In this solution you don't need to enable allow_user_specified_id. Thanks so much.

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

No branches or pull requests

6 participants