Ecto-3 support. - Githubissues

collegeimprovements commented 5 years ago

Are there any plans for Ecto-3 support ?

jbachhardie commented 5 years ago

We don't do a lot of Elixir development around here anymore so this will definitely go quicker if someone from the community steps up and makes a PR but I'll make a note now to try to get around to it if nobody does.

collegeimprovements commented 5 years ago

Thanks a lot.

whossname commented 5 years ago

I took a few months off from this but as of this weekend it is my main project outside of work. I found that migrating this repository to Ecto 3 was a bit difficult, so instead I have created a new repo.

My strategy has been to copy the PostgreSQL adaptor into the new repository. After that I copied the unit tests from this repository into the new one. I am now in the process of getting all of the unit tests to work (mostly by copying code from this repo into the new one). Once the unit tests are working it should be most of the way there, although I'm pretty sure some tests are missing. The last step will be to get the integration tests to work and start testing it out on some production code.

I don't think mssqlex will need the same treatment. The ecto_3.0 branch seems fine so far.

The new repository: mssql_ecto_3

whossname commented 5 years ago

A quick progress report.

I had a bit of a break through this weekend. Finally got the integration tests running. Not passing, but running. Most of these tests are still failing, but getting the integration tests to run is what I got stuck on back in January/February.

I'm actually a bit confused on how they were working on Ecto 2 because the fix was for a mssql feature, not an Ecto feature. As far as I know the syntax for a parameter in a prepared statement should be ?, not ?1. This is the change that got the integration tests to run, but the ?1 notation works in the Ecto 2 version, so I don't know what is happening there.

Anyway, still plenty to do, but it is getting closer.

rafikiadmin commented 5 years ago

Thank you for working on this! I would be willing to help. I will pull down a copy and see if I can at least understand a bit. I have not looked at any Ecto code before, but I would really like to use Ecto with SQL Server AND LiveView, and it seems that 3 support is essential to make it all work together, so I am willing to learn to help.

whossname commented 5 years ago

@rafikiadmin I'm not sure if LiveView changes things, but the Ecto-2 adaptor is definitely usable. Let me know if you need more detailed instructions on how to set it up.

I think I am getting close to getting the Ecto-3 version to work, but even when it does work I'm expecting it to have some fairly significant bugs (mostly around data types).

Anyway this is a pretty big project, so it would be great to get some help. Even if you could lend a hand with the documentation and project management side of things that would be a big help. I'm thinking that most of the documentation could be copied across from this repository, but we will need to pick and choose the most relevant stuff.

rafikiadmin commented 5 years ago

It is very possible that I am misunderstanding something and it has been almost a month since I tried, but it seemed that I could not make version 2 and LiveView work simultaneously. Version 3 of Postgres works just fine, so I thought the conflict was the Ecto version. When I have time this weekend I will try again and let you know the conflict. Maybe you will have insights. Thanks.

whossname commented 5 years ago

Pay really close attention to your dependencies. Ecto 2 and 3 use different versions of db_connection. Also ecto_sql was split out of ecto, so don't use ecto_sql for Ecto 2. I'm pretty sure they use different versions of decimal as well. Make sure to run mix clean -all to make sure you are using the correct dependency versions.

whossname commented 5 years ago

It's been a while so I will give another progress report.

I've been solving a bunch of smaller bugs over the last two weeks and I am happy that the new Ecto 3 adaptor works for simple SQL queries. There are a large number of remaining bugs, but the biggest one is related to using the sandbox. I plan on tackling that next week. Once this issue is resolved I expect to have a much clearer understanding of the remaining work that needs to be done.

I think the next step will be to publish an early version of the new adaptor with the remaining bugs clearly documented. That way even though it is incomplete, at least something will be available for the people who need it.

@jbachhardie I'm expecting this approach will be fine for mssql_ecto_3 because it is a new repository so the semantic versioning can start again from v0.0.0. Do you have any ideas on how to handle it on the mssqlex side? That repository is already at v1.1.0 so incrementing the major version when there are still bugs is a big no-no right? Some of the failing integration tests are definitely related to issues with the new version of mssqlex.

jbachhardie commented 5 years ago

This sounds like a case for releasing a v2.0.0-beta.1. Hex does support the prerelease section of semver, right?

Also, are you planning on keeping mssql_ecto_3 as a separate package? It would probably be nice to keep the mssql_ecto name for the convenience of people using it. I might be able to give you ownership of the name if that's necessary (I'll need to have a chat with the other authors but I don't anticipate any objections we're not using this anymore since we moved to Postgres).

whossname commented 5 years ago

You are right, pre-releases are supported.

I was planning on keeping mssql_ecto_3 as a separate package, but your argument for keeping the existing name makes sense. My main concern with that is merging the two repositories. Git only supports this if one repository has been forked from the other, so instead I would have to copy-past the new version into a branch of mssql_ecto. Not a huge issue, but something to think about.

A strong argument for doing the merge is that it keeps the conversations and history together in the same repository. We don't need to migrate this conversation to an issue in the other repo.

jbachhardie commented 5 years ago

I don't mind having a commit that just rewrites the repo. It will just correctly reflect the discontinuous nature of the development on this.

whossname commented 5 years ago

Ok, cool. I'll do this on the weekend.

whossname commented 5 years ago

this pull request https://github.com/findmypast-oss/mssql_ecto/pull/44

whossname commented 5 years ago

I'm going to take a break from this. Just for a week or two. This is a summary of the main problems that I am aware of at the moment:

integers stored as strings
decimals returned as integers (possibly after being stored as strings)
An error triggered by TSQL's ambiguous "ORDER BY" clause
Several migration errors. We can just say migrations are not supported for these.
A race condition that results in the "No SQL-driver information available." error.
Foreign key errors. These look like they might be caused by the type errors above. A cursory investigation indicates this is not the case.
unique constraint errors
datetime syntax errors. Can't parse 'interval' '1 year', 'three weeks', ect.
'COUNT' syntax error
'IS' syntax error
this error apears a few times "Cannot insert explicit value for identity column in table 'posts' when IDENTITY_INSERT is set to OFF."

collegeimprovements commented 4 years ago

Hi Guys, Any update on this?

whossname commented 4 years ago

It's ready for a beta release with a bunch of known problems and I expect there to be more problems I don't know about yet. I haven't worked on it in the last few months

jbachhardie commented 4 years ago

Thanks for the reminder, just released 2.0.0-beta.0 from #44 for people to use/test

I haven't really tested it since we have no applications running this anymore so at your own risk.

findmypast-oss / mssql_ecto

Ecto-3 support. #43