add map/1 to query api - #4781
Closed
greg-rychlewski wants to merge 1 commit into
Closed
Conversation
Member
Author
|
Now I am unsure actually. This has a pretty bad limitation...you cannot do the from(city in City, preload: :country,
select: %{city | country: map(country: [:id, :population])})Or similarly couldn't do it with virtual fields. It might be surprising as well that it's only the query fields in the resulting map. I am pretty sour on this at the moment but will try to think on it some more. |
Member
Author
|
I have one idea...right now this change is converting the post processing schema to schema = if map_source?, do: nil, else: schema
{{:source, {source, schema}, prefix || query.prefix, types},
This could be changed so that it keeps the schema but then records it's suppose to be a map in the end. And then in the actual post processing step we can keep whichever keys we want. Maybe we want everything except for the metadata fields. Maybe something like |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trying again to introduce
map/1. I think I simplified to a good place in my head but let me try to explain my reasoning. For general strategy I think we have two options:source_take!take: :alland keep the AST the sameThe reason I chose (1) is because in general I think we want to treat this like a source and only care about the map part during post processing. So as much as we can keep the same the better. Also dealing with
take: :allduringselect_mergeadds some complexityNow given I chose (1) I saw two ways to change the AST:
{:map, [], {:&, [], [ix]}}.This PR does (1) but I actually started with (2) and then I noticed it was a bit cumbersome dealing with the different pattern matching in
collect_fieldsand some of the subquery stuff. It's not impossible but it is more than what this current change is doing.Let me know what you think!