Warning: not ready for use! There’s still significant work to do. I hope I will be able to finish it.
lejoko
lejoko
Hi, I opened a PR against ash_sql for a small non standard SQL problem, that MySQL does not support. Moreover, I’m not too sure where to talk about those things. Where would be the right place ?
0
zachdaniel
Creator of Ash
lejoko
Hi again,
I’m almost done with making my ash_mysql module pass all tests that came from ash_sqlite. I’ve a few problems left :
- There are several
exproperators (<>,||and&&) that would require MySQL specific changes inash_sqlin thedo_dynamic_exprfunction that is inlib/expr.exbut that function is not redefinable in the behaviour module and it would seem strange to redefine all operators when only a few need redefinition - I’m not sure how to proceed regarding upserts. MySQL can upsert, but it can’t return the results and I found no way to fake the reloading with upserts (as I did with inserts and updates). Maybe should I leave upserts out ?
- My modules fakes reloading after create or update in a way that can work only if the table has single column primary key that is generated by Ash. At the moment, that behaviour is hard coded. I’m not sure how to proceed regarding that.
- I think it would be great to add transactions and aggregates to ash_mysql. What would be the best way to proceed ?
1
zachdaniel
Creator of Ash
This is very exciting stuff!
- I think we probably do want to allow extensions to override “any” expression, and to do that what we need to do is to rename
do_dynamic_exprtodo_generic_dynamic_expr(only the function heads, not where we call it), and replace thedo_dynamic_exprfunction with something like:
defp do_dynamic_expr(...) do
if get_specific_expr(...) do
{:ok, expr} -> expr
{:error, error} -> {:error, error}
:error -> do_generic_dynamic_expr(....)
end
end
This allows for a SQL extension to “hijack” any expression that requires special processing.
-
You can disable upserts for now by adjusting the
can?/2function. Then we can figure something out later
-
In the short term, I’d require that all primary keys have default values and are marked as
generated?: true. In the long term, we need to work this capability into Ash core, with capabilities likereturn_after_create,return_after_update, etc. This will be work, and will disable various capabilities, but would make us more compatible with various things. -
Adding transactions should be relatively simple, but I’m not familiar with transaction semantics in MySql so it’s hard to say for sure.
aggregates however, are much more complicated Does MySQL support lateral joins?
0
lejoko
-
If I understand you well, that would be a breaking change, requiring all adapters depending on ash_sql to be updated ? Are there other ones than ash_postgres and ahs_sqlite currently ?
-
I’ll go that way for now
-
When you says “require that all primary keys have default values and are marked as
generated?: true”, you mean with no code to guarantee it ? like just explaining that it currently works that way it in the doc ? -
MySQL has no nested transactions but has savepoints. I far as I understand ash_postgres uses that ?
But my question was more like : what needs to be done to support transactions ? Would git grepping ‘transaction’ in thetestsdirectory of ash_postgres, copying them and making them pass be enough ? Can you foresee something that would make that approach insufficient ? -
regarding aggregates, MySQL 8 does supports lateral joins (since 8.0.14, I discovered them recently and they are great!). Currently my module is developped with MySQL 8.0 in mind anyway and has at least one place where it wouldn’t work with older versions without change.
0
zachdaniel
Creator of Ash
-
It shouldn’t be, because they should already explicitly match on certain expressions, returning
:errorif they don’t want to override them. -
Your data layer can add a
verifierthat enforces it, if it is a requirement. ash/lib/ash/resource/verifiers/validate_accept.ex at main · ash-project/ash · GitHub that is an example verifier. Then you’d add it in `use Spark.Dsl.Extension, verifiers: [YourVerifier] -
Yep, copying the transaction semantics from
ash_postgreswould make sense
If MySQL has savepoints then it can also support the error expression potentially, which would make it the “second best” sql data layer in terms of support as it would be able to support atomics & the like. Assuming there is a way to raise a custom error. -
thats awesome news! In that case, you could attempt to use the aggregates code contained in
AshSqland “see what happens” There will 100% be modifications required to make it work with MySQL. I’d suggest that we ship everything else before focusing on this.
This is awesome work. Its shaping up really well and if you can get it to support aggregates, transactions and errors, then this has two major benefits:
- AshSql fully supporting two distinct flavors of SQL, as right now it still has plenty of postgres specific stuff.
- A great new SQL data layer
1
lejoko
Ok, all tests that came from ash_sqlite are now passing. It should currently work almost like ash_sqlite but without upserts and only when using primary keys generated by ash. Primary keys autogenerated by MySQL (AUTO_INCREMENT) should probably also work but not for bulk insert/update. I’ve not yet added a verifier to enforce those restrictions. Up to now, I’ve worked only on making tests pass, so I’ve not really tested the module in a real app. I’m just now starting to play with it in an app in order to “feel” how it works.
Moreover, I’ve had a look at how ash_postgres does transactions (and custom errors) and it looks like it should be doable with MySQL. Hopefully I’ll be able to add it later. I’ve not yet had a serious look at aggregates.
If my tests go well (and when an official ecto_sql release include my fix) I will probably make a first release. Would you (@zachdaniel) like to have ash_mysql as an “official” part of ash ? After all, it is still mostly your code (from ash_sqlite), my changes are not the biggest part… Anyway, I’ve left almost all of the ash project “branding” that came from ash_sqlite in the repo for now. I’d be happy if it could be “absorbed” officially in the ash project. What is your view on that ? I’m not sure which way to go in this regard.
4
zachdaniel
Creator of Ash
This is awesome! I’d be very happy to have this under our umbrella, but it would be important to keep your name associated with all of the work you’ve done. Likely best way would be to transfer the repo to the ash org, and I can add you as a maintainer(if you like).
0
lejoko
I added my name to the Copyright in the MIT License file. I’d happily remain a maintainer for now (hopefully the module is in its infancy). I tried transferring the repo to the ash-project org but github says that
“You don’t have the permission to create public repositories on ash-project”.
I could transfer it to you and then you’d transfer it to your org if you want.
0