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

UUID support (primary keys with default value and crypto extension) #187

Open
eraffel-MDSol opened this issue Nov 5, 2022 · 10 comments
Open

Comments

@eraffel-MDSol
Copy link

Why is the PK excluded when setting default values on the columns of the views?

next if column.name == pk || default.nil?

@tagliala
Copy link
Member

tagliala commented Nov 5, 2022

I can't tell

Is there something your application is trying to achieve that it is not possible when using CM?

@eraffel-MDSol
Copy link
Author

It's failing to insert into the table, it's saying the PK that is being inserted (NEW.id) is null.

@eraffel-MDSol
Copy link
Author

I also don't understand what you meant by "I can't tell", the line I linked to skips adding default values to the PK column in the VIEW.

@tagliala
Copy link
Member

tagliala commented Nov 5, 2022

It's failing to insert into the table, it's saying the PK that is being inserted (NEW.id) is null.

Can I have a reproducible test case?

I also don't understand what you meant by "I can't tell", the line I linked to skips adding default values to the PK column in the VIEW.

Sorry, not a native English speaker. I can't tell exactly why it is being excluded. My guess is that primary key are often automatically generated and for all our use cases it is fine

@eraffel-MDSol
Copy link
Author

You can clone this, populate the database.yml, run db setup, and run the rspec. You'll see the error

@eraffel-MDSol
Copy link
Author

https://github.com/eraffel-MDSol/chrono-issue-reproduction
whoops, forgot the link

@eraffel-MDSol
Copy link
Author

Looks like there's maybe not support for a uuid PK?

@tagliala
Copy link
Member

tagliala commented Nov 5, 2022

Looks like there's maybe not support for a uuid PK?

Oh, yes, this could be the reason.

I've recently implemented support for string IDs #156, but we do not use UUIDs

We are interested in supporting UUID, but at the moment this feature is out of scope.

Feel free to take a look and propose a PR with test coverage

@evman182
Copy link

evman182 commented Nov 7, 2022

I opened #188

Sorry, keep switching between work and personal account

@tagliala
Copy link
Member

tagliala commented Nov 7, 2022

Thanks, it looks good, I would like to test it a little bit more before merging

@tagliala tagliala changed the title PK default values excluded from VIEW UUID support (primary keys with default value and crypto extension) Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants