[WIP] Support for Ecto 2.0 #91

ankhers · 2016-08-10T18:37:05Z

Just copying the list from #84

This is a list of the remaining tests that need to pass before the Ecto 2.0 cut can be made.

test/mongo_ecto_test.exs:26

This is a list of tests that will not be fixed and why.

deps/ecto/integration_test/cases/repo.exs:68 - Mongo does not officially support composite keys.
deps/ecto/integration_test/cases/repo.exs:91 - Mongo does not officially support composite keys.
deps/ecto/integration_test/cases/repo.exs:281 - Mongo does not support joins.
deps/ecto/integration_test/cases/repo.exs:306 - Mongo does not support joins.
deps/ecto/integration_test/cases/repo.exs:557 - Mongo does not support sub queries.
deps/ecto/integration_test/cases/repo.exs:585 - Mongo considers null (or no value) to be the same across a unique index.
deps/ecto/integration_test/cases/repo.exs:625 - Mongo considers null (or no value) to be the same across a unique index.
deps/ecto/integration_test/cases/repo.exs:890 - The mongodb package is making an explicit decision to only return string keys. I'm going to follow this because of the lack of schema associated with Mongo.
deps/ecto/integration_test/cases/preload.exs:80 - Mongo does not support joins.
deps/ecto/integration_test/cases/preload.exs:199 - Mongo does not support joins.
deps/ecto/integration_test/cases/preload.exs:267 - Mongo does not support joins.
deps/ecto/integration_test/cases/preload.exs:410 - Mongo does not seem to support true transactions.
deps/ecto/integration_test/cases/preload.exs:483 - Mongo does not seem to support true transactions.
deps/ecto/integration_test/cases/type.exs:64 - Mongo does cannot cast types in a query.
deps/ecto/integration_test/cases/type.exs:86 - Mongo does cannot cast types in a query.
deps/ecto/integration_test/cases/type.exs:94 - ^"a" in ^["a", "b", "c"] does not translate well into Mongo.

fix update_all and insert_all tests

ankhers · 2016-08-19T15:28:11Z

For anyone that is following along, I have to make a couple changes in the mongodb package that will make development on this adapter be a little smoother.

* Mongo does not support returning * Mongo does not support joins * Mongo does not support composite keys

hartator · 2016-08-21T14:46:50Z

Awesome! Do you need more help? Seems there still are a lot to do.

ankhers · 2016-08-21T14:56:36Z

I would appreciate any help you or anyone else could provide. I currently
have a couple commits that I have not yet pushed. I should be able to push
them within an hour or two.

After that, just try and leave a message here with what you are working on
to try and prevent duplicate efforts.

On Aug 21, 2016 10:46, "hartator" notifications@github.com wrote:

Awesome! Do you need more help? Seems there still are a lot to do.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#91 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAt0wj-y9atzYMntwhVqdqnSUNyRsa15ks5qiGTcgaJpZM4JhcfN
.

hartator · 2016-08-21T15:07:17Z

Sure, will be glad to find some time to help.

coveralls · 2016-08-23T14:51:05Z

Changes Unknown when pulling b5853b8 on ankhers:ecto-2 into * on michalmuskala:ecto-2*.

Mongo wants you to be explicit about not wanting the ID field returned

coveralls · 2016-08-29T15:39:31Z

Changes Unknown when pulling c2c2144 on ankhers:ecto-2 into * on michalmuskala:ecto-2*.

coveralls · 2016-08-31T01:00:46Z

Changes Unknown when pulling eeb0da0 on ankhers:ecto-2 into * on michalmuskala:ecto-2*.

cjbell · 2016-09-13T18:02:32Z

@ankhers I'm very happy to take logging as I just ran into an issue with it and started to fix!

scottmessinger · 2016-09-13T18:11:03Z

@cjbell That would be awesome! I also ran into the logging issue today.

cjbell · 2016-09-13T18:57:29Z

@scottmessinger if you need something working right now, you can point to this branch (which is a fork of @ankhers ecto-2 branch): https://github.com/madebymany/mongodb_ecto/tree/ecto-2-log-fix

ankhers · 2016-09-13T18:58:57Z

You can also configure your repository with log: false to prevent logging. Which is what I am using in a project at the moment. But I would appreciate an actual fix.

justmark · 2017-01-22T21:05:07Z

@ankhers

Thanks for the super quick response! I added in the log: false, and that did the trick. The error isn't reported, and all the data passed through to the table as I had hoped (very quickly, too.)

AlexKovalevych · 2017-01-22T21:31:29Z

@justmark in case you need logging, you can check this PR, which works for me: #8

justmark · 2017-01-22T21:35:56Z

@AlexKovalevych Thanks - I'll give that a shot too. Thanks for the reply, its much appreciated.

Fix logging

ankhers · 2017-02-07T15:18:28Z

Just as a quick update to everyone, it appears that the only remaining test to fix is the JavaScript test.

If someone has any argument(s) for fixing the tests that I have decided to not fix (you can see the entire list in the PR description) please let me know. If you change my mind, or provide a PR, I will gladly attempt to fix it.

michalmuskala · 2017-02-07T16:02:55Z

That's an awesome news. I'll review the code over this week (or weekend).

I'll also do appropriate changes to ecto to make it possible to isolate the tests that require features MongoDB does not provide.

Thank you, @ankhers for all the work you did here.

ankhers · 2017-02-07T21:21:00Z

As it turns out, I'm not entirely sure about making the javascript test pass. where/2 requires a keyword list, and you cannot introduce keywords without making changes to ecto proper.

Is this something that people are even interested in using?

scottmessinger · 2017-02-08T04:02:44Z

@ankhers I never use javascript in my mongo queries as it can't make use of any indexes, so for me, the lack of where/2 isn't a big deal at all. That said, I could see it being valuable for some. If there isn't any demand now, perhaps just make a github issue and let future collaborators know what they'd need to do to make those tests pass.

merqlove · 2017-02-10T18:49:25Z

Seems here wrong search, when we searching by map. For example:

Repo.get_by(Some, map_field: %{})

It will search with map_field: [].
So result will be empty.

merqlove · 2017-02-10T20:33:57Z

I have added spec for this case:
cnsa@18dae11

Seems it is more Mongo/MongoDB behavior.

ankhers · 2017-02-11T16:01:57Z

@merqlove I do not think that is an issue. I was able to insert and query properly based on map input.

iex(5)> Repo.insert(%Schema4{map1: %{"foo" => "bar"}, binary: "binary"})
{:ok,
 %Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">, binary: "binary",
  id: "589f3466b3acd935e72ce13e", map1: %{"foo" => "bar"}}}
[debug] QUERY OK db=35.4ms
INSERT coll="schema4" document=[binary: #BSON.Binary<62696e617279>, map1: %{"foo" => "bar"}, _id: #BSON.ObjectId<589f3466b3acd935e72ce13e>] []

iex(6)> Repo.all(Schema4)
[debug] QUERY OK db=0.2ms
FIND coll="schema4" query=[{"$query", []}, {"$orderby", %{}}] projection=%{_id: true, binary: true, map1: true} []
[%Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">, binary: "binary",
  id: "589f3466b3acd935e72ce13e", map1: %{"foo" => "bar"}}]

iex(7)> Repo.insert(%Schema4{binary: "binary2"})
[debug] QUERY OK db=0.4ms queue=0.1ms
INSERT coll="schema4" document=[binary: #BSON.Binary<62696e61727932>, map1: %{}, _id: #BSON.ObjectId<589f3487b3acd935e7d35197>] []
{:ok,
 %Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">,
  binary: "binary2", id: "589f3487b3acd935e7d35197", map1: %{}}}

iex(8)> Repo.all(from s in Schema4, where: s.map1 == ^%{})
[debug] QUERY OK db=0.2ms
FIND coll="schema4" query=[{"$query", [map1: %{}]}, {"$orderby", %{}}] projection=%{_id: true, binary: true, map1: true} []
[%Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">,
  binary: "binary2", id: "589f3487b3acd935e7d35197", map1: %{}}]

iex(9)> Repo.all(from s in Schema4, where: s.map1 == ^%{"foo" => "bar"})
[debug] QUERY OK db=0.2ms
FIND coll="schema4" query=[{"$query", [map1: %{"foo" => "bar"}]}, {"$orderby", %{}}] projection=%{_id: true, binary: true, map1: true} []
[%Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">, binary: "binary",
  id: "589f3466b3acd935e72ce13e", map1: %{"foo" => "bar"}}]

iex(10)> Repo.get_by(Schema4, map1: %{})
[debug] QUERY OK db=0.2ms
FIND coll="schema4" query=[{"$query", [map1: %{}]}, {"$orderby", %{}}] projection=%{_id: true, binary: true, map1: true} []
%Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">, binary: "binary2",
 id: "589f3487b3acd935e7d35197", map1: %{}}

iex(11)> Repo.get_by(Schema4, map1: %{"foo" => "bar"})
[debug] QUERY OK db=0.4ms
FIND coll="schema4" query=[{"$query", [map1: %{"foo" => "bar"}]}, {"$orderby", %{}}] projection=%{_id: true, binary: true, map1: true} []
%Schema4{__meta__: #Ecto.Schema.Metadata<:loaded, "schema4">, binary: "binary",
 id: "589f3466b3acd935e72ce13e", map1: %{"foo" => "bar"}}

ericmj · 2017-02-11T17:25:08Z

@merqlove Can you show a failing test with Repo.get_by?

This is not mongodb behaviour:

iex(1)> %{foo: %{}} |> BSON.encode |> BSON.decode
%{"foo" => %{}}
iex(2)> %{foo: []} |> BSON.encode |> BSON.decode
%{"foo" => []}

ericmj · 2017-02-11T17:26:22Z

It is possible that the adapter translates maps to keyword lists. If it does that it should not do it for empty maps.

merqlove · 2017-02-11T19:11:18Z

@ericmj @ankhers
I have added 2 new specs.
First is failed, second with success.
cnsa@04853cb
Also here gist with illustration.
i did gist https://gist.github.com/merqlove/981c93faa52b751c64a538c147a2e25d

This is not critical moment, but better to have maps more predictable.

ankhers · 2017-02-11T19:49:54Z

Okay. When I showed this as working, I didn't realize but the project was using an older commit. This appears to be a regression. I will take a look at where this broke.

ankhers · 2017-02-11T20:29:15Z

I found the problem (It is the most recent commit). So I should be able to basically revert it and things should work as expected.

But I would prefer to just not convert the empty map into an empty list.

merqlove · 2017-02-11T20:36:36Z

@ankhers great!
now i'm integrating our app into these changes, i will write if found anything else.

ex_machina works ok with this upgrade.
scrivener paginator will not work at our side anymore because they use subquery. So kerosene :)

michalmuskala · 2017-02-12T21:07:55Z

Thank you @ankhers for working on this.

I'm going to prepare for a release in #84.

ankhers · 2017-02-13T00:14:09Z

@merqlove Just so you know, I have fixed that issue. Though, going forward, you may want to switch to the official ecto-2 branch (that is where this fix is).

merqlove · 2017-02-13T03:53:55Z

Excellent, thanks!

justmark · 2017-02-20T00:55:50Z

@AlexKovalevych Any chance you could post some of your code to get the pipeline working? I added it in, and it doesn't toss any errors, but doesn't appear to log any request to Mongo at all.

merqlove · 2017-02-20T04:04:43Z

@justmark do you have debug logs? because i have no problem with it (as it was before upgrade).

AlexKovalevych · 2017-02-20T08:42:47Z

@justmark if you mean aggregation pipeline, then it doesn't log those i think, since i use mongo driver directly, ecto has nothing to do with it. Aggregation example:

    Mongo.aggregate(YourRepo.__mongo_pool__, collection_name, [%{"$match" => ..}, %{"$group" => ..}])

justmark · 2017-02-20T15:50:51Z

@AlexKovalevych Thanks - I've got:

cursor = Mongo.aggregate(Svr.Repo.__pool__,  "systems", pipeline )

but that results in an error:

no match of right hand side value: []

I also tried using your "mongo_pool" suggestion, but that also generated an error:

function Svr.Repo.__mongo_pool__/0 is undefined or private

merqlove · 2017-02-20T16:02:00Z

@justmark Seems you have wrong pipeline.
I have no error in such query:
Mongo.aggregate(Repo.Pool, collection_name, [%{"$sample": [size: size]}], [])

justmark · 2017-02-20T16:14:07Z

@merqlove Hi, I saw in your query that you used "Repo.Pool". I used this instead of what I had previously (Svr.Repo.pool) and your change worked.

Thanks!

ankhers and others added 7 commits August 9, 2016 13:33

Loaders and dumpers for time and date types

95f8c13

Fix a couple more tests

a2e8c47

Fix regex queries

874d619

Comment out the javascript test for now

4a2f069

fix update_all and insert_all tests

fce0159

Merge pull request #1 from appunite/ecto-2

0eaaaa3

fix update_all and insert_all tests

Revert back to ericmj repo

0f23fa0

ankhers mentioned this pull request Aug 17, 2016

[WIP] Ecto 2 compatibility #84

Closed

Fix-up some tests

e72965a

* Mongo does not support returning * Mongo does not support joins * Mongo does not support composite keys

ankhers added 3 commits August 22, 2016 13:13

Test fix for in

3392180

Fix a couple tests for update and delete

c2d26b9

Following ecto's versions for travis

b5853b8

ankhers added 2 commits August 29, 2016 10:12

Fix selecting only explicit attributes

e5f5686

Mongo wants you to be explicit about not wanting the ID field returned

Implement ensure_all_started for Ecto.Adapter

c2c2144

Temporarily point to my fork for mongodb

eeb0da0

bAmpT mentioned this pull request Aug 31, 2016

Is Ecto 2 supported? #92

Closed

Revert back to upstream mongo repo

b3d1bba

michalmuskala mentioned this pull request Sep 16, 2016

How to use Elixir ~> 1.3 #93

Closed

michalmuskala mentioned this pull request Sep 28, 2016

Hex dependency resolution failed #94

Closed

Merge pull request #8 from AlexKovalevych/fix-logging

6ed4a28

Fix logging

maennchen mentioned this pull request Jan 26, 2017

Failed to use "ecto" (versions 2.1.0 to 2.1.3) #105

Closed

Cleaner way of checking for empty map

39c08cb

michalmuskala merged commit 6efdb8a into elixir-mongo:ecto-2 Feb 12, 2017

ankhers mentioned this pull request Feb 16, 2017

Ecto 2.1 Compatibility #109

Closed

[WIP] Support for Ecto 2.0 #91

[WIP] Support for Ecto 2.0 #91

Conversation

ankhers commented Aug 10, 2016 • edited Loading

ankhers commented Aug 19, 2016

hartator commented Aug 21, 2016

ankhers commented Aug 21, 2016

hartator commented Aug 21, 2016 • edited Loading

coveralls commented Aug 23, 2016 • edited Loading

coveralls commented Aug 29, 2016 • edited Loading

coveralls commented Aug 31, 2016 • edited Loading

cjbell commented Sep 13, 2016

scottmessinger commented Sep 13, 2016

cjbell commented Sep 13, 2016

ankhers commented Sep 13, 2016

justmark commented Jan 22, 2017

AlexKovalevych commented Jan 22, 2017

justmark commented Jan 22, 2017

ankhers commented Feb 7, 2017

michalmuskala commented Feb 7, 2017 • edited Loading

ankhers commented Feb 7, 2017

scottmessinger commented Feb 8, 2017

merqlove commented Feb 10, 2017

merqlove commented Feb 10, 2017 • edited Loading

ankhers commented Feb 11, 2017 • edited Loading

ericmj commented Feb 11, 2017

ericmj commented Feb 11, 2017

merqlove commented Feb 11, 2017

ankhers commented Feb 11, 2017

ankhers commented Feb 11, 2017 • edited Loading

merqlove commented Feb 11, 2017 • edited Loading

michalmuskala commented Feb 12, 2017

ankhers commented Feb 13, 2017

merqlove commented Feb 13, 2017

justmark commented Feb 20, 2017

merqlove commented Feb 20, 2017

AlexKovalevych commented Feb 20, 2017

justmark commented Feb 20, 2017

merqlove commented Feb 20, 2017

justmark commented Feb 20, 2017

ankhers commented Aug 10, 2016 •

edited

Loading

hartator commented Aug 21, 2016 •

edited

Loading

coveralls commented Aug 23, 2016 •

edited

Loading

coveralls commented Aug 29, 2016 •

edited

Loading

coveralls commented Aug 31, 2016 •

edited

Loading

michalmuskala commented Feb 7, 2017 •

edited

Loading

merqlove commented Feb 10, 2017 •

edited

Loading

ankhers commented Feb 11, 2017 •

edited

Loading

ankhers commented Feb 11, 2017 •

edited

Loading

merqlove commented Feb 11, 2017 •

edited

Loading