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

Rebase of #1871: Support diesel_manage_updated_at on SQLite #1955

Merged
merged 2 commits into from Jan 14, 2019

Conversation

kaj
Copy link
Contributor

@kaj kaj commented Jan 13, 2019

This PR is a rebase of #1871 . See that one for motivation and discussion.

Code by @sgrif , I have just done a rebase (manual but obvious merge was needed in CHANGELOG.md and diesel/src/sqlite/connection/raw.rs).

sgrif and others added 2 commits January 13, 2019 23:25
There's a little bit of funkiness required to make this work. The first
piece is that we need to modify the function registration to provide
access to the connection for our internal functions. This isn't
*strictly* necessary, we could `transmute` the `&self` to be `&'static
self`, since the function will never be called after `self` is dropped.
That felt like it would add some unneccessary unsafety though.

The second bit of funkiness is that we have to have *some* return type,
so we can't just return `()`. I think I want to fix this in the future
by providing a `Null` SQL type, which provides `ToSql` and `FromSql`
impls only for `()`. This requires adding a variant to `SqliteType` and
`MysqlType` though, so it will have to be done in 2.0.

The function itself is subtly different from the PostgreSQL version,
since it runs even if no values were actually changed (we can't do
something like `NEW IS DISTINCT FROM OLD` here). This also cannot be run
on tables without ROWIDs. I think we should probably declare this as a
SQL function in code somewhere for documentation purposes, but I want to
wait until our docs are building again to follow up with that.
Trying to make CI happy.
@weiznich weiznich requested a review from a team January 14, 2019 09:32
Copy link
Member

@weiznich weiznich left a comment

Choose a reason for hiding this comment

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

Closes #1871

@weiznich weiznich merged commit 420d86e into diesel-rs:master Jan 14, 2019
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.

None yet

3 participants