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

Update to Ecto 3 #24

Open
wants to merge 47 commits into
base: master
Choose a base branch
from

Conversation

whossname
Copy link

@whossname whossname commented Aug 4, 2019

Description

An early beta version of mssqlex for the mssql_ecto migration to Ecto 3.

Motivation and Context

The result of my efforts to try and update mssql_ecto to Ecto 3

How Has This Been Tested?

Updated the unit tests to use db_connection 2.0 and modified them until they passed. Additionally the integration testing described in this pull request to mssql_ecto.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@matreyes
Copy link

Hi, it's failing when joining multiple tables and using column and table aliases. @whossname, hope you can fix it !

@whossname
Copy link
Author

whossname commented Sep 13, 2019 via email

@whossname
Copy link
Author

@matreyes the code from two commits ago should address that. It might fail if columns from different tables have the same name.

We are hoping to have a beta available soon. What is your time frame for needing this?

@whossname
Copy link
Author

whossname commented Sep 14, 2019

@matreyes I just added this test for the situation you are describing, and in my branch at least it is working fine. Could you please elaborate on what the issue is and if possible suggest a better test?

@matreyes
Copy link

@whossname It works great now, thanks for the fast response!

@whossname
Copy link
Author

@jbachhardie this is ready for review. I think it should be basically ready to publish.

@jbachhardie
Copy link

Thank you for all your hard work and apologies for not getting to this yet I'll book out some time to review/publish it next week.

@whossname
Copy link
Author

I've been playing around a bit and it seems like the "No SQL-driver information available." triggers when you try to delete data and htere is nothing to delete. This may not be the only case where this error is triggered, but I think it is a bit of a clue as to why it is happening.

@nikneroz
Copy link

@whossname @jbachhardie Hey! Do you need some help? I have some time this month and it will be great to finish this PR and push it to hex :)

@whossname
Copy link
Author

I would suggest that the way forward is to depreciate this repository in favour of TDS

https://github.com/livehelpnow/tds

@whossname
Copy link
Author

@nikneroz this is way harder than it looks unless you have a deep understanding of both Ecto and Transact SQL. I wasn't up to the task needed way more time and effort than I had available. That TDS repo should work fine though.

@nikneroz
Copy link

I would suggest that the way forward is to depreciate this repository in favour of TDS

https://github.com/livehelpnow/tds

Ok, thank you!

@nikneroz
Copy link

@nikneroz this is way harder than it looks unless you have a deep understanding of both Ecto and Transact SQL. I wasn't up to the task needed way more time and effort than I had available. That TDS repo should work fine though.

I'll check the current state and will try to figure out what do we have at this moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants