vladmihalcea / hypersistence-optimizer

Hypersistence Optimizer allows you to get the most out of JPA and Hibernate. By scanning your application configuration and mappings, Hypersistence Optimizer can tell you what changes you need to do to speed up your data access layer.
https://vladmihalcea.com/hypersistence-optimizer/
Apache License 2.0
318 stars 43 forks source link

Optional OneToOne should not be used with MapsId #23

Closed eximius313 closed 5 years ago

eximius313 commented 5 years ago

Let's say we have two entities: User and ActivationToken ActivationToken is created together with User creation, but it must survive (for legal purposes) after User deletion. Therefore:

CREATE TABLE users (
  id BIGINT PRIMARY KEY,
  email VARCHAR(254)
);

CREATE TABLE activation_tokens (
  id BIGINT PRIMARY KEY,
  user_id BIGINT REFERENCES users(id) ON DELETE SET NULL,
  ip VARCHAR(20)
);

And

class User {
  @OneToOne(mappedBy = "user", fetch = FetchType.LAZY, optional = true, orphanRemoval = false,
      cascade = {CascadeType.PERSIST, CascadeType.DETACH})
  private ActivationToken token;
}

class ActivationToken {
  @JoinColumn(name = "user_id", nullable = true, unique = false)
  @OneToOne(fetch = FetchType.LAZY, optional = true)
  private User user;
}

I think that in such situation MapsId should not be used because we can not SET NULL primary key, and having id withoult FOREIGN KEY constraint doesn't look good either, but Hypersistence Optimizer throws:

21:24:18.894 [pool-1-thread-1] [error] Hypersistence Optimizer - CRITICAL - OneToOneWithoutMapsIdEvent - The [user] one-to-one association in the [model.ActivationToken] entity is using a separate Foreign Key to reference the parent record. Consider using @MapsId so that the identifier is shared with the parent row. For more info about this event, check out this User Guide link - https://vladmihalcea.com/hypersistence-optimizer/docs/user-guide/#OneToOneWithoutMapsIdEvent
vladmihalcea commented 5 years ago

That's a one-to-many table relationship according to the database. The parent mapping is also going to cause performance issues unless bytecode enhancement is used, so the warning is valid.

You can always filter events that you think are fine for your application.

eximius313 commented 5 years ago

It's not one-to-many. It's missing unique because there can be only one null in unique index. It's optional one-to-one (see nullable=true and optional=true). Shouldn't Hibernate treat it differently?

vladmihalcea commented 5 years ago

It is one-to-many from a database perspective. You should enforce constraints at DB level, not in your application or via annotations.