Update to master

[whossname]

• merged in master
• added TDS_PATH environment variable for development
• fixed existing tests
• early attempts to get the integration tests working
• Update to the new NaiveDateTime implementation

Looks like there are a bunch of minor white space changes from running the formatter aswell. Didn't notice those until latter.

Over the last year I've been working on updating the mssql adaptor by findmypast to Ecto 3. At a technical level this implementation makes more sense, the other one relies on the Erlang odbc library, which isn't actively supported and seems to cause some type issues. I thought I would have look to see how far along this library is. So far it looks like this library has a bunch of the same issues I was experiencing with that library:

• Decimals drop the trailing ".0" (I thought I solved this one but can't remember the details)
• A large number of the integration test problems seem to be because of how MSSQL handles locks. This causes problems with migrations and transactions.
• unique constraint issues
• streams not implemented
• This test fails in exactly the same way:

  test aggregate (Ecto.Integration.RepoTest)
   deps/ecto/integration_test/cases/repo.exs:744
   match (=) failed
   code:  assert "12.5" <> _ = to_string(TestRepo.aggregate(Post, :avg, :visits))
   right: "12"
   stacktrace:
     deps/ecto/integration_test/cases/repo.exs:757: (test)

• Another one I've seen before, not sure if I solved it: Cannot insert explicit value for identity column in table 'posts' when IDENTITY_INSERT is set to OFF.

There are a bunch of other issues that I'm only seeing in one or the other of the two libraries though.

Specific to this library:

• Implicit conversion from data type nvarchar to varbinary is not allowed. This seems to be resolved by replacing "varbinary" with "nvarchar" for the :binary type. Not sure if this is an appropriate solution though.
• issue in subqueries where nil appears as the first element in the expr tuple
• Repo.all returns a json string instead of a map (not sure of the details). Code:

  assert [%{"reference" => "EXAMPLE"}] = TestRepo.all(from(o in "orders", select: o.item()))

• union not working (not sure of the details)
• "data types text and nvarchar are incompatible"
• "Incorrect syntax near the keyword 'NOT'"
• :conflict_target is not supported
• :on_conflict is not supported
• select_merge doesn't seem to be supported (not sure of the details)
• protocol Enumerable not implemented for nil of type Atom. Happens with this code:

  TestRepo.update_all(select("posts", [:id, :title]), [set: [title: "x"]])

• insert all not supported? (not sure of the details)
• non-boolean type specified in a context where a condition is expected
• date_to_gregorian_days function clause error (in interval tests)

There are others I didn't understand well enough to add to these lists.

[mjaric]

Hi @whossname thanks for your effort.

I already started working again on this branch few days ago, and I have mostly same fixes locally except for datetime2, datetimeoffset and time sql types, and I'm still struggling to avoid adding custom types since precision for those types is 7 digits and elixir has 6 which could be an issue for some users. I'm still thinking how to avoid precision loss without implementing ecto custom types and I will probably revert back datetime decoder in TDS to erl tuple representation so loaders and dumpers can do they work for orm part. So in case of custom type it will have it own struct and for ecto builtin types it will be loaded into elixir builtin types.

Regarding :binary it should be mapped to varbinary so integration_tests should be changed rather than remapping this type to nvarchar. For nvarchar we already have ecto's :string type. As far as I can tell for other adapters, they have flexibility in db backend to compare binary with strings since they are utf-8 encoded binary like the erlang string :) MS has different encoding which mostly is utf-8 compatible but it is always 2 bytes long (ucs2 that MS refers in docs is older unicode encoding version).

Findmypast had similar issues since nature of how ecto tries to simplify parameters passing by allowing it to be erlang builtin type in query and then guess by consulting adapter type mapping what that sql type should be, but the list also limited comparing to what mssql has to offer. Most of other orms or drivers do it by requiring parameter argument wrapped in special struct/type that should say in what db type it should be converted before it sent to db backend. An example of my attempt to properly support varchars is by tagging ecto parameter e.g. {:varchar, "some text"} with custom Tds.Types.Varchar, but it is custom type and it makes integration_test useless in such case so I need to add more just for this adapter.

:map/JSON support, is kept as string since JSON is added in MSSQL after version 2012 and driver was built even before 2012. Some companies that uses ecto are still mssql 2012 consumers so changing this is not good idea at the moment. BTW i changed unit test to cover this, I'll push it later to repo.

Again, thanks for your support and please don't be disappointed since I will close this PR without merging it. As I said I already have most of this locally ad it will be in repo anytime soon.

TNX MJ

[whossname]

The fixes included in this pull request were pretty obvious, so it's not surprising that you already had them locally. I'll have another look when you push the changes.

Cheers.