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

add :deleted column to any schema that uses alog #5

Open
3 tasks
nelsonic opened this issue Oct 23, 2018 · 3 comments
Open
3 tasks

add :deleted column to any schema that uses alog #5

nelsonic opened this issue Oct 23, 2018 · 3 comments
Labels
enhancement New feature or request starter

Comments

@nelsonic
Copy link
Member

nelsonic commented Oct 23, 2018

As a developer creating Phoenix Web Apps/APIs using alog,
I would like to have the :deleted column added to each schema/table where alog is used.
So that I don't have to manually add the :deleted column
or think about how to "delete" records.

Todo

  • automatically add a :deleted field to a schema when the use AppendOnlyLog is employed.
  • Write tests for schema/field creation
  • Document this in the README.md so it's clear to people why this column is needed/added.

as initially discussed in dwyl/phoenix-ecto-append-only-log-example#7 (comment)

@Danwhy
Copy link
Member

Danwhy commented Oct 23, 2018

After looking into this, it doesn't appear to be possible to modify a schema without completely redefining the Ecto.Schema function.
The most we can do is throw an error, or display a warning if the required field is not present in the schema.

The same is true of the :deleted column in the database, although there is a conventional way that we could add it here, which is by turning this module into an Ecto Adapter, which would define its own migration module. This would likely involve a lot of work though, would significantly change the API, and probably wouldn't be different enough from the standard Postgres Adapter to be worth it.

@nelsonic What do you think? Is adding a warning that a deleted field is needed enough?

@nelsonic
Copy link
Member Author

@Danwhy good question, if we are unable to "inject" the additional fields into the schema
perhaps we can group all the alog related fields at the end of the schema definition?

A warning would be a good checkpoint to help remind people using alog.

For now avoid the Ecto Adapter work unless it will make the use of alog significantly easier for other devs implementing this.

@Danwhy
Copy link
Member

Danwhy commented Oct 24, 2018

Thinking a little further, it might be good to actually throw an error rather than just a warning, as most of the functions won't actually work without a :deleted field.

While there may be some use cases that don't actually need to use a :deleted column, and maybe we should eventually look at making it possible to not require it, for now we'll leave it in and throw an error if it doesn't exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request starter
Projects
None yet
Development

No branches or pull requests

2 participants