quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.74k stars 2.67k forks source link

Panache Rx #2198

Closed FroMage closed 3 years ago

FroMage commented 5 years ago

Formal issue to discuss Panache Rx implementation.

Branch is at https://github.com/FroMage/quarkus/tree/panache-rx

Proposal is the same API as Hibernate ORM with Panache, but Reactified, with Axle.

List of JPA annotations supported

Proposed set of supported JPA annotations: (✗ = no support, ✓ = already supported, R = Required, N = Nice, ? = no idea yet, N/A = not relevant).

Annotation Member Support
@AssociationOverride
@AssociationOverrides
@AttributeOverride
@AttributeOverrides
@Basic
@Column
name
unique N
nullable N
insertable N
updatable N
columnDefinition N/A
table
length N
scale N
@ColumnResult
@Converter N
@DiscriminatorColumn
@DiscriminatorValue
@Embeddable N
@Embedded N
@EmbeddedId N
@Entity
name N
@EntityListeners
@EntityResult
@Enumerated
@ExcludeDefaultListeners
@ExcludeSuperclassListeners
@FieldResult
@GeneratedValue
strategy ✓(only SEQUENCE)
generator
@Id
@IdClass N
@Inheritance
@JoinColumn ✓(for many-to-many)
@JoinColumns
@JoinTable ✓(for many-to-many)
@Lob N
@ManyToMany
targetEntity ?
cascade ?
fetch LAZY
mappedBy
@ManyToOne
targetEntity ?
cascade ?
fetch LAZY
@MapKey
@MappedSuperclass
@NamedNativeQueries
@NamedNativeQuery
@NamedQueries
@NamedQuery
@OneToMany
targetEntity ?
cascade ?
fetch LAZY
mappedBy R
@OneToOne
targetEntity ?
cascade ?
fetch LAZY
mappedBy
@OrderBy N
@PersistenceContext
@PersistenceContexts
@PersistenceProperty
@PersistenceUnit
@PersistenceUnits
@PrimaryKeyJoinColumn
@PrimaryKeyJoinColumns
@QueryHint
@SecondaryTable
@SecondaryTables
@SequenceGenerator
@SequenceGenerators
@SqlResultSetMapping
@SqlResultSetMappings
@Table
name
catalog R
schema R
uniqueConstraints ?
@TableGenerator
@Temporal
@Transient
@UniqueConstraint ?
@Version

Types supported:

Type Support
String
primitives/wrappers
byte[]
Byte[]
char[]
Character[]
java.util.Date
java.util.Calendar
java.sql.Date
java.sql.Time
java.sql.Timestamp
javax.time.LocalTime
javax.time.LocalDate
javax.time.LocalDateTime
javax.time.OffsetTime
javax.time.OffsetDateTime
enum (to string)
enum (to int)
Serializable
Embedded
tsegismont commented 5 years ago

@FroMage since #2039 is all green, could the implementation use the reactive-pg-client extension?

FroMage commented 5 years ago

That's my plan, I'm trying to make my branch use your module ATM.

tsegismont commented 5 years ago

Great!

soberich commented 5 years ago

This would be first such an ability on the market, I think. Right now the only decision non-JPA implementation which allow to map Entities with JPA annotations, generate schema, migrate, support SQL 2011 History, triggers, etc is Ebean. Ebean uses plain JDBC (again with JPA annotations - which is very nice) and it's QueryBeans are absolutely the best (alike Panache). But of course those are standard blocking drivers. On the other hand non-blocking clients are so far from maturity, that more in state of experiments. They support no mapping and, of course, R2DBC does not support nesting at all (thought your implementation while using whether R2DBC or vertx client might not support it either). Such feature will be absolutely the best. Schema generation is also quite valuable. Watching on this one. Thank you.

FroMage commented 5 years ago

OK guys, it's possible this is ready for review and trials.

I support already the full test of panache orm, plus most basic types and many of the most common annotations. I believe the most annoying missing so far is cascade and embedded.

I'd like people to try it out and tell me what annotations they're missing to use this, and why.

emmanuelbernard commented 5 years ago

I did a review of the panache-rx code. No concrete comment as I was trying to understand it mostly. I did not see any patently wrong things. I might do a second pass with more concrete feedback. Note that scaling to other DBs, and more complex domain model will likely explode the code complexity. Good job, you've successfully unlocked "Wrote ones own ORM" :)

emmanuelbernard commented 5 years ago

The biggest missing features I see in decreasing order:

  1. ability to load an entity and (some of) its associations in one query
    • the CompletableFuture or Publisher execution would then be a no DB access operation
  2. object inheritance if only discriminator column
  3. @Version
  4. HQL
  5. @Embeddable
  6. @Converter and otherwise composite types

I understand that 1. and 4. are a lot of work. I think 1. is a deal breaker for any non trivial app.

On a side note, I think Hibernate with Panache could benefit from query projection with some object filling. But that's a different story even though that might be the deal breaker patch.

FroMage commented 5 years ago

ability to load an entity and (some of) its associations in one query

I guess I could do eager loading for some cases, based on the association type, but given that I don't support HQL I can't specify eager fetching in HQL, so how useful would that make it?

object inheritance if only discriminator column

I was hoping this was never actually used. I can take a look at how much work that is

@Version

This should not be hard

FroMage commented 5 years ago

So the fun thing is that I looked at the spec to see how to implement @Version and looked at https://dzone.com/articles/hibernate-collections as a result, and realised that there are a lot of variations of @OneToMany/@ManyToOne that I don't support ATM, such as unidirectional or owning @OneToMany relations.

My personal opinion is that those relation types are to be avoided anyway, but perhaps they're also important? In any case it does mean that I was optimistic in claiming I supported @OneToMany.

FroMage commented 5 years ago

@Version is now supported

FroMage commented 5 years ago

@emmanuelbernard can you confirm that eager fetching is useful even when I can't specify it from the HQL? It will only be usable from the annotations.

I do get how it's useful for @*ToOne associations, even though I'll probably limit it to one-level deep fetching initially.

I don't understand why it's useful for @*ToMany associations because lazy fetching uses two queries: one for the entity (single row) and one for its collection (multiple rows). Eager fetching with a join uses a single query (multiple rows) so one query less, at the cost of many more columns per row and so probably worse throughput from the DB. This gets even worse if there are more than one such relation involved.

Can you explain why that would be useful in practice?

FroMage commented 5 years ago

About inheritance mappings, I think the table per class strategy is a no-go from the start because the whole model revolves around a top class of PanacheEntity which has an id column, so this would force multiple tables even for entities that don't really want inheritance.

As for the single table per hierarchy with discriminator column strategy it would essentially mean every entity is stored in a single class again due to the top PanacheEntity class.

As such, I would stay away from advising this, much less implement it here.

emmanuelbernard commented 5 years ago

ability to load an entity and (some of) its associations in one query

I guess I could do eager loading for some cases, based on the association type, but given that I don't support HQL I can't specify eager fetching in HQL, so how useful would that make it?

No, the fact that associations are lazy by default is fine. Overloading is really on a use case basis and thus somehow needs to be expressed per query. Maybe with https://docs.jboss.org/hibernate/orm/5.4/userguide/html_single/Hibernate_User_Guide.html#fetching-strategies-dynamic-fetching-entity-graph

object inheritance if only discriminator column

I was hoping this was never actually used. I can take a look at how much work that is

Discriminator is the easiest one.

@Version

This should not be hard

The last guy who said that was found dead in a pyramid. ahahah

emmanuelbernard commented 5 years ago

So the fun thing is that I looked at the spec to see how to implement @Version and looked at https://dzone.com/articles/hibernate-collections as a result, and realised that there are a lot of variations of @OneToMany/@ManyToOne that I don't support ATM, such as unidirectional or owning @OneToMany relations.

My personal opinion is that those relation types are to be avoided anyway, but perhaps they're also important? In any case it does mean that I was optimistic in claiming I supported @OneToMany.

It's ok to skip unidirectional @OneToMany

emmanuelbernard commented 5 years ago

@emmanuelbernard can you confirm that eager fetching is useful even when I can't specify it from the HQL? It will only be usable from the annotations.

I do get how it's useful for @*ToOne associations, even though I'll probably limit it to one-level deep fetching initially.

I don't understand why it's useful for @*ToMany associations because lazy fetching uses two queries: one for the entity (single row) and one for its collection (multiple rows). Eager fetching with a join uses a single query (multiple rows) so one query less, at the cost of many more columns per row and so probably worse throughput from the DB. This gets even worse if there are more than one such relation involved.

Can you explain why that would be useful in practice?

See my other reply the @ToOne/Many annotations are not where it should land. You are semi right:

I agree with your assessment to focus initially on the ToOnes

emmanuelbernard commented 5 years ago

About inheritance mappings, I think the table per class strategy is a no-go from the start because the whole model revolves around a top class of PanacheEntity which has an id column, so this would force multiple tables even for entities that don't really want inheritance.

As for the single table per hierarchy with discriminator column strategy it would essentially mean every entity is stored in a single class again due to the top PanacheEntity class.

As such, I would stay away from advising this, much less implement it here.

Hum I did not realize that. Why isn't PanacheEntity a @MappedSuperclass

FroMage commented 5 years ago

Hum I did not realize that. Why isn't PanacheEntity a @MappedSuperclass

It is, otherwise it would not get mapped, right? But that doesn't change my argument, does it? It would still require its own table or a discriminator column just for the id.

emmanuelbernard commented 5 years ago

No @MappedSuperclass are not represented in the Entity hierarchy (no discriminator, no separate table etc). Specifically the entity extending @MappedSuperclass will just carry its properties as if they were hosted on the entity itself.

FroMage commented 5 years ago

Aha, OK, then I misunderstood it and it remains a valid option.

misl commented 4 years ago

I am curious what the timeline is for this one?

emmanuelbernard commented 4 years ago

We are exploring implementing Panache RX on top of what we call Hibernate RX. Due to the big 1.0 effort this was put to hold but has now been restarted. So stay tuned but I can't give you an estimate.

manoelcampos commented 4 years ago

Has anyone been able to use @Version? It works fine with plain Hibernate. After including Panache, there is no OptimisticLockException when multiple users try to update the same record on the DB.

I don't know if the issue is related to the way I'm performing the update. A sample class is below:

@Entity
public class City extends PanacheEntity implements Serializable {
    public String name;

    @Version
    public long version;

    public static boolean update(City city) {
        City existing = City.findById(city.id);
        if(existing != null){
            existing.name = city.name;
            existing.version = city.version;
            existing.persist();
            return true;
        }

        return false;
    }
}

A complete example with plain Hibernate and Panache is available at https://github.com/manoelcampos/hibernate-concurrency

FroMage commented 4 years ago

Can you open another issue please. This issue is unrelated.

manoelcampos commented 4 years ago

Hello @FroMage, Sorry, I saw people discussing about @Version here and thought it was the right place to report. I just created issue #7193.

hakandilek commented 4 years ago

Any updates on this one? It's been over 4 months since the last update from @emmanuelbernard.

FroMage commented 4 years ago

We'll have something for preview in a matter of weeks :)

cescoffier commented 3 years ago

@FroMage should we close this issue?

FroMage commented 3 years ago

Oh yes.