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

Order shape elements before decoding #380

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Conversation

aljazerzen
Copy link
Contributor

@aljazerzen aljazerzen commented Feb 3, 2025

Make it so Queryable on structs does not require the struct to have the same order of fields as the shape elements in the EdgeQL query.

This is done by capturing the order of the shape elements in check_descriptor and then applying that order in decode.

@aljazerzen aljazerzen changed the title order shape elements Order shape elements before decoding Feb 3, 2025
@aljazerzen aljazerzen changed the base branch from master to decode-args February 3, 2025 10:44
@aljazerzen
Copy link
Contributor Author

aljazerzen commented Feb 3, 2025

Since Queryable::Args is specified for each type separately, fields of a Queryable struct might also emit args, which need to be passed to their decode call. This is currently not implemented or even tested.

Edit: fixed (for shapes) with Handle sub-args in Queryable for structs

Base automatically changed from decode-args to master February 3, 2025 10:48
@aljazerzen
Copy link
Contributor Author

Closes #285

@aljazerzen aljazerzen force-pushed the order-shape-elements branch from 22de18c to 79694a8 Compare February 3, 2025 10:48
@aljazerzen aljazerzen requested a review from msullivan February 3, 2025 11:10
Copy link
Member

@msullivan msullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mostly able to follow this, but the macro definitions really could use comments that show what they generate.

@aljazerzen
Copy link
Contributor Author

Yeah. All macro code I've written is like that. Not saying that it cannot be different, I'm realizing that maybe I should do something about it. Comments are a good start.

@aljazerzen aljazerzen merged commit 907fb3b into master Feb 4, 2025
4 checks passed
@aljazerzen aljazerzen deleted the order-shape-elements branch February 4, 2025 08:19
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.

2 participants