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

orm: fix subquery without where expr #21598

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

felipensp
Copy link
Member

Fix #21595

@juan-db
Copy link
Contributor

juan-db commented May 29, 2024

I think there's a bigger problem here. At some point deep select was implemented so select statements will automatically retrieve children as well, but as soon as you have a deeper nested relationship, you would get the panic from the original issue. I think the ORM feature needs a way for you to explicitly load children.

@spytheman
Copy link
Member

spytheman commented May 30, 2024

I think there's a bigger problem here. At some point deep select was implemented so select statements will automatically retrieve children as well, but as soon as you have a deeper nested relationship, you would get the panic from the original issue. I think the ORM feature needs a way for you to explicitly load children.

Please file a separate issue for that, with an example.

The PR afaik works as it is, the changes in it seem logical to me, and it does fix the problem described in your original issue. The only thing it lacks to no longer be a draft, is a test to prevent silent regressions in the future.

#0 17:53:44 ^ fix_subexpr_where /v/oo>v run z.v
[Parent{
    id: 1
    name: 'first parent'
    children: [Child{
        id: 1
        name: 'first child'
        parent_id: 1
        babies: []
    }, Child{
        id: 2
        name: 'second_child'
        parent_id: 1
        babies: []
    }]
}]

@felipensp felipensp marked this pull request as ready for review May 31, 2024 12:41
@juan-db
Copy link
Contributor

juan-db commented May 31, 2024

My main concern is the where_expr field actually does have a value (id ==), it's just not a valid value. This likely points to a logic error somewhere.

@juan-db
Copy link
Contributor

juan-db commented Jun 3, 2024

I've created a new issue so I'm happy with having this PR merged when ready.

@medvednikov medvednikov merged commit 1e86e06 into vlang:master Jun 4, 2024
76 checks passed
@medvednikov
Copy link
Member

Merged, thanks.

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 this pull request may close these issues.

ORM select compile-time panic with deep relationships
4 participants