-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: insert embedded resources via computed columns #3226
base: main
Are you sure you want to change the base?
Conversation
95be53c
to
3419b9f
Compare
d7c3354
to
85ad53e
Compare
dfdb39f
to
739af4e
Compare
It seems that #2933 is definitely going to be an issue when -- "tags" is modified inside "upsert_posts"
-- but the LEFT JOIN will show the OLD unmodified "tags"
SELECT * FROM upsert_posts(...) p
LEFT JOIN tags t ON p.id = t.pid; It could be solved by embedding the RETURNING value of the function, but it cannot be further embedded. For example: Here, the curl "http://localhost:3000/posts?columns=name,tags(name)?select=*,tags(name) \
-H "Content-Type: application/json" \
-H "Prefer: return=representation" \
-d '{"name": "my post", "tags": [{"name": "a"},{"name": "b"}]}' [{
"name": "my post",
"tags": []
}] Making curl "http://localhost:3000/posts?columns=name,tags(name) \
-H "Content-Type: application/json" \
-H "Prefer: return=representation" \
-d '{"name": "my post", "tags": [{"name": "a"},{"name": "b"}]}' [{
"name": "my post",
"tags": [{"name": "a"},{"name": "b"}]
}] |
739af4e
to
eb69214
Compare
eb69214
to
fdeffd5
Compare
This is because it is in a transaction. Since PostgREST uses CTEs, it can't return an after value before the transaction has been committed. I think all functions are transactional by default. Theoretically, you could write a nesting I think allowing nested data is more important than it returning the nested value. I'm not going to have a Perhaps a work around would be an option to the select statement like J |
Yes, I agree, this is the priority right now. Once we complete this, we can worry on how to return the data. Also the |
Side note on this, I was asking Edge DB how they would handle the return, and they would have to do a query like this: with user := (
insert User {
username := 'tim'
}
),
todos := {(
insert Todo {
text := 'one',
created_by := user
}
), (
insert Todo {
text := 'two',
created_by := user
}
)},
select user {
todos := select todos {
id,
text
}
}; Although I'm not sure what this EdgeQL compiles to in postgres. Note sure if this helps any though process. This is inserting a user with a todo in example. J |
This is not about transactions, but about snapshots. All CTEs in a single top-level statement run with the same snapshot, that's why they can't see modifications of other CTEs, except for via RETURNING. So there does not need to be a COMMIT, just a new statement to see the changes.
Yeah, this is the way to go. Note that we not only need to fetch the RETURNING data, but possibly also the already existing data, so we might need UNIONs there. Example: [{
"name": "my post",
"tags": [{"name": "a"},{"name": "b"}]
}] Assume that tag "a" already exists and tag "b" does not, yet. The relation insert will:
To return the full response, we'll then need to embed both tags again, so we will need a UNION between |
One more note on this:
I think it's in fact the opposite: The relational insert is the solution to this problem, if we do it right. If we support all the |
Food for thought... There are different situations that need to be considered that I talked about in the original post:
Also, there could be the option to work like an array:
@wolfgangwalther - Could you give an example of what J |
More of that here: #818 (comment) |
That clarified it for me too. That would solve the main issue I was getting while working on this feature. So now I realize that my approach of implementing the virtual columns first is not the best one here (the design can be reused later though). I'll continue with the relational inserts right now. |
WIP