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

Implement Append Only Log as an Ecto Adapter #38

Open
7 of 17 tasks
Danwhy opened this issue Feb 6, 2019 · 2 comments
Open
7 of 17 tasks

Implement Append Only Log as an Ecto Adapter #38

Danwhy opened this issue Feb 6, 2019 · 2 comments
Assignees
Labels
enhancement New feature or request epic help wanted Extra attention is needed

Comments

@Danwhy
Copy link
Member

Danwhy commented Feb 6, 2019

We should look into implementing this module as an Ecto Adapter.

This will allow us greater control over the internals of the Ecto Repo, including allowing us to autogenerate a cid as the primary key, hopefully allowing us to require or predefine required fields such as :deleted and timestamps, and ensuring there are no unique indexes.

We are currently doing some of this, but only by taking advantage of macros. By implementing an Ecto Adapter, we can use the module in the exact way we would use the normal Postgres Adapter, so the learning curve will be minimal for anyone who has used Ecto before.

We can take advantage of the already existing functionality from Ecto.Adapters.SQL, re-implementing what we need to change, and keeping what we don't.

We should start with a simple tutorial explaining how to create an Ecto Adapter, then build on that.

https://michal.muskala.eu/2015/07/07/creating-ecto-adapters.html
http://blog.plataformatec.com.br/2019/01/building-a-new-mysql-adapter-for-ecto-part-iv-ecto-integration/

The following are the callbacks we need to define. Some of them we will be able to defdelegate to the existing Ecto.Adapters.Postgres module. There may also be others which are already defined as part of Ecto.Adapters.SQL but that we need to modify (some of the Ecto.Adapter.Queryable callbacks for example). We'll add those to the list as it becomes apparent we need them.

Ecto.Adapters.SQL.Connection

Ecto.Adapter.Storage

  • storage_down(options) - defdelegate
  • storage_up(options) - defdelegate

Ecto.Adapter.Migration

  • supports_ddl_transaction?() - defdelegate or just return true
@Danwhy Danwhy added the enhancement New feature or request label Feb 6, 2019
@RobStallion
Copy link
Member

I have been looking into approaches that we could take to 'convert' alog into an ecto adapter.

After looking at the postgres implementation, I think we will be able to extend the existing functions, even ones that are not quite what we want/need "out of the box".

Let's look at the all function as an example...
The current alog.all/0 function simply calls repo.all/1 with the following query...

sub =
  from(m in __MODULE__,
    distinct: m.entry_id,
    order_by: [desc: :updated_at],
    select: m
  )

query = from(m in subquery(sub), where: not m.deleted, select: m)

@repo.all(query)

from converts the arguments into an ecto struct and then passes that struct to Repo.all/1 (as alog is currently being used, it passes in a the query struct to the postgres adapter. We can see an example of this here).

Ecto adapters work by receiving an etco query struct as an argument and pattern matching on the fields to create a query string.

I think that rather than rewriting the logic for each of the callbacks ourselves, we should use the existing postgres adapter functions but update the parameters in the query struct before we call the postgres adapter.

An example of this... (this is not meant to be a working example atm, just a way to make the point more clear)

def all(query) do
  sub =
    from(m in __MODULE__,
      distinct: m.entry_id,
      order_by: [desc: :updated_at],
      select: m
    )

  alog_query = from(m in subquery(sub), where: not m.deleted, select: m)

  new_query = Map.merge(query, alog_query) # (way over simplified but the idea would be to update the original query passed in with the alog query)

  Ecto.Adapters.Postgres.Connection.all(new_query)
end

Now I do not expect this function to work as it is (just to be clear) but hopefully it helps explain my above point. Under the hood, the variables query and alog_query are structs. I don't see any reason why we cannot update the parameters in the query struct that is passed into our all/1 function to "alog-ify" it.

This way we can create an adapter that uses the postgres adapter to create the query strings we want. Essentially we will just be adding to the query that is passed in to make it work how we would like alog to work.

We would also be able to have function clauses that returned an error to the user if we didn't want them to use a certain query function. For example, if we did not want to allow users to call distinct/3 (as this may clash with the alog distinct call) we could have a clause that pattern matched for distinct and returned a helpful error message to the user explaining what went wrong.

@Danwhy @SimonLab @nelsonic Please let me know if you think it is something that is worth looking into? Any thoughts and feedback on this would be greatly appreciated.

@RobStallion RobStallion added help wanted Extra attention is needed question Further information is requested labels Feb 11, 2019
@Danwhy
Copy link
Member Author

Danwhy commented Feb 12, 2019

Definitely worth looking into. I get the feeling it will be slightly less efficient, but it should be easy to refactor if we manage to get it at least working for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request epic help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants