Skip to content

Conversation

zentourist
Copy link
Contributor

This was a fun one! Compound Primary Keys almost broke me 😆 I started thinking this would take a day or two. I was wrong...

What this does:

  • Changes the internal format of the Ash.Resource records when they get stored in Mnesia so that we can better query against the values using Matchspecs
  • Builds MatchSpecs from Ash.Filter so that we can do in-Mnesia filtering. Today this is just using guards. I would like to eventually add Pattern Matching in the MatchHead for even more efficient queries.

It does NOT support all Filter functionality and if we get into any issues it will simply fallback to runtime filtering for now.

I added comprehensive tests for the Matchspec items. The tests were largely generated with Sonnet 4.5, but I reviewed them in depth.

Initially, this broke a bunch of tests in the suite, but I was able to get everything working again without touching any of the existing testing so feature-wise it should be backward compatible. NOTE: The internal format does change so if there are any persisted tables out there THIS WILL BREAK THEM.

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@zachdaniel
Copy link
Contributor

🤔 NOTE: The internal format does change so if there are any persisted tables out there THIS WILL BREAK THEM.

We are going to need to figure something out here then, like making these new changes backwards compatible. Alternatively, we could break out a new data layer called AshMnesia for example, so that we can iterate on breaking changes.

iex> #{inspect(__MODULE__)}.mnesia_record_info()
[:id, :name, :age, :email]
"""
def mnesia_record_info do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As opposed to defining functions inside the resource, we should use functions in Ash.DataLayer.Mnesia.Info, and use the transformer to put this information into the persisted data, i.e Spark.Dsl.Transformer.persist

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 - I'll see if I can figure this out. Are you talking about just this function or reorganizing them all this way? If so, I might need an example to point to.

@@ -0,0 +1,253 @@
defmodule Ash.DataLayer.Mnesia.MatchSpec do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool! I think this actually dovetails into something I'd really like to exist for this, however, which is something called Ash.Filter.split, which you pass in a filter, and a function that says "do you support this expression element" and it gives you back {ands_that_you_support, ors_that_you_dont}, that way you can use the first thing to build your own filter statement, and the second thing can be done with runtime filtering at the end.

Would you be open to this? This is huge BTW.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a cool solution. 🆒

I think that would definitely clean up the logic a bit and then we wouldn't have to do an all or nothing approach that we are here. My only concern is, will the filters split that way naturally. Or is there fallout if we do the split and run some in the datalayer and some at runtime?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concept with splitting is that you only split across *and* boundaries. i.e

a == 1 and (b == 2 or has(c, "value"))

You'd get back

{expr(a == 1), expr(b == 2 or has(c, "value"))}.

So the left filter can always be applied, and the right is applied afterwards.

@zentourist
Copy link
Contributor Author

🤔 NOTE: The internal format does change so if there are any persisted tables out there THIS WILL BREAK THEM.

We are going to need to figure something out here then, like making these new changes backwards compatible. Alternatively, we could break out a new data layer called AshMnesia for example, so that we can iterate on breaking changes.

Yeah, I've been noodling on this. I don't hate the idea of breaking it out, but it is nice to have a good data_layer in the core for testing. I noticed a few tests use the mnesia DL. We could also just add a V2 namespace. That would also allow for easy benchmarks between the two :)

I also had a thought about introducing migrations using transform_table, but I haven't spent any time digging into that yet.

@zentourist
Copy link
Contributor Author

I had one additional thought on the internal table format. I might be able to support both versions by introducing something like this:

mnesia do
  table :table_name
  version 1
end

If someone uses v1 for the format, it would work in a backward compatible way and allow for deprecation. I'd have to give this a go, but I think it's possible.

@zachdaniel
Copy link
Contributor

Yep, I think versioning makes a lot of sense. We can potentially introduce a backwards compatibility configuration (see the docs on backwards compatibility configs) to make new apps default to the new better version, or make the option required (or warn when unset) so that users will have to configure it and can decide how to handle it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants