luckyframework / avram

A Crystal database wrapper for reading, writing, and migrating Postgres databases.
https://luckyframework.github.io/avram/
MIT License
164 stars 64 forks source link

Default columns should generate values from postgres #393

Open jwoertink opened 4 years ago

jwoertink commented 4 years ago

If you're using something like Hasura to connect through Lucky, the GraphQL may need to insert records in to postgres at some point. If you happen to have Avram setup with UUID, then your ID, and the timestamps are all generated from Crystal. This means that inserting a record through GraphQL fails.

To temporarily fix this, you can create a migration that runs this:

def migrate
  enable_extension "uuid-ossp" # NOTE: this requires Avram > 0.15? (as of today, it isn't released yet)
  execute("ALTER TABLE users ALTER COLUMN id SET DEFAULT uuid_generate_v4();")
  execute("ALTER TABLE users ALTER COLUMN created_at SET DEFAULT CURRENT_TIMESTAMP;")
  execute("ALTER TABLE users ALTER COLUMN updated_at SET DEFAULT CURRENT_TIMESTAMP;")
end

/cc. @russ @KCErb

KCErb commented 4 years ago

@jwoertink thanks for finding this and the fix! I've been planning to take advantage of Live Queries/subscriptions from GraphQL and not so much mutations so I wouldn't have caught this for a while (I think). Thank you!

jwoertink commented 4 years ago

Yeah, @russ is actually the one that found it. He's using your lucky-hasura deal. 😃

KCErb commented 4 years ago

Really? That's great I had no idea!

Thanks @russ please share your notes with me as bump into rough edges. You're apparently now more experienced than I am in my own project 😲 !

russ commented 4 years ago

@KCErb haha. Not really but your repo is helping me learn all of this stuff. The only other issue I've run into is cors related stuff but I can submit a pull request for that on your repo. I'll let you know over there if I run into anything else.

paulcsmith commented 4 years ago

@jwoertink The only thing missing is the updated_at needs a trigger and function to set the timestamp when the row is updated.

Also I think the UUID is now generated by default, but if not I agree it should be 👍

matthewmcgarvey commented 4 years ago

Just as a note uuid-ossp has a note in their docs:

If you only need randomly-generated (version 4) UUIDs, consider using the gen_random_uuid() function from the pgcrypto module instead.
matthewmcgarvey commented 4 years ago

Just wanted to drop this link here while I happen to know where to find it, rails' handling of uuid as primary key setup https://github.com/rails/rails/blob/580eefe1bc5d7241a4b2f3ee863523c5b8fd2f80/activerecord/lib/active_record/migration/compatibility.rb#L123-L128

matthewmcgarvey commented 3 years ago

UUID primary keys now have defaults in the database with https://github.com/luckyframework/avram/pull/546

the timestamps are the next to consider

matthewmcgarvey commented 3 years ago

I just looked into Rails and Laravel. Neither of them set a default value in the database for the created_at and updated_at columns. They are both set in the code.

jwoertink commented 3 years ago

I still think this could be a neat feature, but we can hold off on doing it since there's an easy work around. I'm actually working on a Lucky project now that will u se GraphQL. I'll see quickly if it becomes a huge pain or not.