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 helpers for dealing with timestamps #91

Closed
sgrif opened this Issue Jan 13, 2016 · 9 comments

Comments

Projects
None yet
2 participants
@sgrif
Member

sgrif commented Jan 13, 2016

created_at doesn't need anything from us, we can just have DEFAULT current_timestamp. updated_at requires a trigger, but we shouldn't require our users to write that. We can use the same procedure for pretty much every table:

CREATE FUNCTION set_updated_at() RETURNS trigger AS $$
BEGIN
    IF (
        NEW IS DISTINCT FROM OLD AND
        NEW.updated_at IS NOT DISTINCT FROM OLD.updated_at
    ) THEN
        NEW.updated_at := current_timestamp;
    END IF;
    RETURN NEW;
END
$$ LANGUAGE plpgsql;

We should provide that so users don't have to write it repeatedly

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 13, 2016

Unsure if this is too much, but I think I'd also like to have something like diesel_manage_updated_at(table_name), because I can never remember the trigger syntax.

@robertmaloney

This comment has been minimized.

Contributor

robertmaloney commented Jan 30, 2016

Did you want to set updated_at upon INSERT as well?

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 30, 2016

We can handle that via DEFAULT NOW()

@robertmaloney

This comment has been minimized.

Contributor

robertmaloney commented Jan 30, 2016

Alright, and you wanted a procedure like this?

CREATE OR REPLACE FUNCTION diesel_manage_updated_at(_tbl regclass) RETURNS VOID AS $$
BEGIN
  EXECUTE format('CREATE TRIGGER set_updated_at BEFORE INSERT OR UPDATE ON %s FOR EACH ROW EXECUTE PROCEDURE set_updated_at()', _tbl);
END;
$$ LANGUAGE plpgsql;
@sgrif

This comment has been minimized.

Member

sgrif commented Jan 30, 2016

I guess it should just be BEFORE UPDATE if we assume updated_at has a default.

@robertmaloney

This comment has been minimized.

Contributor

robertmaloney commented Jan 31, 2016

Oh yeah, had that one written prior to your response haha. Apologies if this is obvious, but where do you imagine these helpers existing? Should the updated_at/created_at -> Timestamp pairs be reserved and handled with this procedure automatically within the migration code, or exposed to the user so they can decide whether or not diesel should manage it for them? Or is there a plan to implement a migration DSL in Rust, akin to Rails (e.g. add_timestamps)?

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 31, 2016

There's currently no Rust DSL, though we may add one in the future as use cases arise. I'm fine with creating this as part of the migrations code, as long as it has a sufficiently specific name.

The actual code for creating these should probably live on the Connection, so we can easily adapt it for SQLite once #152 is merged.

@robertmaloney

This comment has been minimized.

Contributor

robertmaloney commented Jan 31, 2016

I've been looking into what exactly would it take to implement this, constrained by the SQL format of migrations. If they wanted to, a user could write valid SQL with alternating capitals and one word per line, and it'd be accepted by Pg. So implementing a check for the phrase updated_at TIMESTAMP and then parsing the table_name would most likely mean collapsing all whitespace, and then using the regex crate's capture groups.

I also thought about a pseudo-DSL using SQL comments, but a good design of that basically means implementing the foundation of a DSL entirely.

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 31, 2016

We shouldn't do any sort of magic around this, the user should set the trigger themselves. We just need to provide the function for them.

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