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

Overriding primary key #8

Open
jensjeflensje opened this issue Aug 26, 2023 · 1 comment
Open

Overriding primary key #8

jensjeflensje opened this issue Aug 26, 2023 · 1 comment

Comments

@jensjeflensje
Copy link

jensjeflensje commented Aug 26, 2023

How would one override the predefined automatically incrementing integer primary key?
In my case, I'd like to use a UUID and I don't need an integer primary key at all.

Example:

@Getter
@Setter
@Column(keyType = KeyType.PRIMARY)
private UUID id;

That fails with: cannot override getId() in com.craftmend.storm.api.StormModel.

I'm thinking the id could be optional by having a base model (without id) and a default model (with id). I could use the base model for models with custom primary keys.

@Mindgamesnl
Copy link
Owner

Mindgamesnl commented Sep 1, 2023

Hey there!

This is definitely worth looking into, though I don't think that #9 is a clean way to go about it because it doesn't force certain implementation traits (like the default getter, type coercion, or presence).

I'll take a look at this concept later this weekend, specifically using class type parameters to define a specific key type, whilst still providing the base getter. This would make it accessible through normal calls and doesn't require additional reflection to access (what should be) a generic field.

There are also some additional issues with the current implementation, mostly concerning serialization. IMO Storm should force a dedicated type adapter to be present for any given value, to ensure that it gets drilled won to its minimum SQL definition (and provide a character limit/encoding type that best fits the boxed type). Otherwise, nothing would be stopping you from using a BigInt as a primary key, which would resolve to a text/varchar instead of a numeric type. Due to this, and a few other reasons (like upstream API changes from return types, due to no other reason than personal preference), the PR will unfortunately not be merged.

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

No branches or pull requests

2 participants