Dzoukr / Dapper.FSharp

Lightweight F# extension for StackOverflow Dapper with support for MSSQL, MySQL, PostgreSQL, and SQLite
MIT License
374 stars 35 forks source link

Implementation of custom operations may not be overloaded #42

Closed tskj closed 3 years ago

tskj commented 3 years ago

Hey guys, I was looking to join on multiple columns and found issue 36. I actually want a left outer join on multiple columns, not an inner join, but that is besides the point.

When I try to upgrade to a later version than 2.1.0 it breaks all of my existing (working) code, with the following error message:

The custom operation 'orderBy' refers to a method which is overloaded. The implementations of custom operations may not be overloaded.F# Compiler(3087)

It appears the F# compiler does not allow overloads for custom operations, and after version 2.1.0 this seems to be how you have implemented both more orderBy stuff and the aforementioned innerJoin on multiple columns. I really appreciate these new features, but how am I supposed to use it?

JordanMarr commented 3 years ago

Did you add the <LangVersion>preview</LangVersion> to the PropertyGroup of your fsproj file per Roman's suggestion on #41?

Also, have you considered using the Linq API?

tskj commented 3 years ago

Oh I hadn't seen that, adding preview actually does seem to resolve it, but I am not quite sure how I feel about that. How does having a preview version of the language fix a compiler issue?

I would have used the Linq API if that was what was shown in the docs. Do you consider it superior? In that case I would recommend updating the docs to reflect that.

JordanMarr commented 3 years ago

It's common for new features to be staged as preview only features.

The Linq API has a major advantage over the base API in that it uses strongly typed records to represent tables and record properties to represent columns. This means that it's impossible to accidentally misspell a table or column in your query, and the mistakes that would be caught at run-time (like entering your join columns in the wrong order) will instead be caught at design-time.

In fact, the base API will likely be removed in favor of the Linq API in v3.0.

Also, there are no overloaded operators in the Linq API, so you won't need the preview tag. 🙂

Dzoukr commented 3 years ago

I would have used the Linq API if that was what was shown in the docs

There's a whole section in README for it + link at the feature list, but for v3.0 we will likely prefer LINQ only and drop (or move to a separate place) old API + docs

preview has been used for more than 18 months and F#5 features are just too great to be not used - e.g. overload of custom operations makes the API way lighter than before

Dzoukr commented 3 years ago

Sorry, the "new" (announced in March 2020) F# features are the way to go and we should not stick to a worse API only because of compatibility issues (which can be solved by fsproj flag). If you are against using this flag then please use version < 2.1.0 - the major functionality has not been changed and also I am not aware of any major bugfix so this version is still fine to use.

tskj commented 3 years ago

Thank you, no I agree overloading is a great feature and works very well with this api! It was just kind of strange that it needed an obscure flag, and what is worse, trying to google the error message I got (before I knew of the flag) sent me to old F# discussion threads where it was argued that overloading like that would never be supported and wasn't a good idea. I disagree with that obviously and think overloading here is a good idea and I'm happy seeing it move into 5. But it was a very confusing ride trying to debug what the hell was going on.

I can also see that the Linq syntax might be better, and I would surely have used it if that was the predominant form in the examples / docs, but that wasn't the case when I first started this project and I'm not keen on rewriting all my queries. Although static guarantees make a good case!