Skip to content

Move preprocessing out of adapters #2312

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

Merged
merged 1 commit into from
Nov 20, 2017
Merged

Conversation

fishcakez
Copy link
Member

Simplify execute (and stream) by removing processing argument. This is a breaking change but it is possible to get backwards compatibility from Ecto 2.2 for an adapter by defining a default preprocessing argument as nil or fn x -> x end depending on the adapter logic.

def execute(repo, query_meta, query, params :: list(), process \\ fn x -> x end, options)

I am not aware of a noSQL adapter that has supported #2155 and this PR builds on top of that in a way.

This will fix the log entry result for SQL drivers. It would be possible to do this by just changing the SQL adapter.

Simplify execute (and stream) by removing processing argument
def query([], _assocs, _sources), do: []
def query(rows, [], _sources), do: rows
def query([], _assocs, _sources, _fun), do: []
def query(rows, [], _sources, fun), do: Enum.map(rows, fun)
Copy link
Member Author

Choose a reason for hiding this comment

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

We would be able to delay this map until next step (preloading) to minimize traversals. However if Ecto controls all processing perhaps we can do better.

@fishcakez fishcakez merged commit 0f4f75f into master Nov 20, 2017
@fishcakez fishcakez deleted the jf-internal-preprocess branch November 20, 2017 04:13
@AndrewDryga
Copy link
Contributor

I guess it will be a ring thing to mention @elixir-ecto/adapters here

@michalmuskala
Copy link
Member

You're right @AndrewDryga. I also added @ankhers to the team since he took over the maintenance of the mongodb adapter.

bartekupartek pushed a commit to bartekupartek/ecto that referenced this pull request Mar 19, 2019
Simplify execute (and stream) by removing processing argument
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.

4 participants