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

Register PGobject subtypes for reflection #33300

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ void build(BuildProducer<ReflectiveClassBuildItem> reflectiveClass) {
final String driverName = "org.postgresql.Driver";
reflectiveClass.produce(ReflectiveClassBuildItem.builder(driverName).build());

// We want to register these postgresql "object" types since they are used by a driver to build result set elements
// and reflection is used to create their instances. While ORM might only use a `PGInterval` if a @JdbcType(PostgreSQLIntervalSecondJdbcType.class)
// is applied to a Duration property, we still register other types as users might create their own JdbcTypes that
// would rely on some subtype of a PGobject:
final String[] pgObjectClasses = new String[] {
"org.postgresql.util.PGobject",
"org.postgresql.util.PGInterval",
"org.postgresql.util.PGmoney",
"org.postgresql.geometric.PGbox",
"org.postgresql.geometric.PGcircle",
"org.postgresql.geometric.PGline",
"org.postgresql.geometric.PGlseg",
"org.postgresql.geometric.PGpath",
"org.postgresql.geometric.PGpoint",
"org.postgresql.geometric.PGpolygon",
yrodiere marked this conversation as resolved.
Show resolved Hide resolved
// One more subtype of the PGobject, it doesn't look like that this one will be instantiated through reflection,
// so let's not include it:
// "org.postgresql.jdbc.PgResultSet.NullObject"
};
reflectiveClass.produce(ReflectiveClassBuildItem.builder(pgObjectClasses).build());

// Needed when quarkus.datasource.jdbc.transactions=xa for the setting of the username and password
reflectiveClass.produce(ReflectiveClassBuildItem.builder("org.postgresql.ds.common.BaseDataSource").constructors(false)
.methods().build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.IOException;
import java.io.PrintWriter;
import java.time.Duration;
import java.util.List;
import java.util.UUID;

Expand Down Expand Up @@ -135,6 +136,7 @@ private static void persistNewPerson(EntityManager entityManager, String name) {
person.setName(name);
person.setStatus(Status.LIVING);
person.setAddress(new SequencedAddress("Street " + randomName()));
person.setLatestLunchBreakDuration(Duration.ofMinutes(30));
entityManager.persist(person);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package io.quarkus.it.jpa.postgresql;

import java.time.Duration;

import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
Expand All @@ -10,6 +13,9 @@
import jakarta.persistence.NamedQuery;
import jakarta.persistence.Table;

import org.hibernate.annotations.JdbcType;
import org.hibernate.dialect.PostgreSQLIntervalSecondJdbcType;

@Entity
@Table(schema = "myschema")
@NamedQuery(name = "get_person_by_name", query = "select p from Person p where name = :name")
Expand All @@ -19,6 +25,7 @@ public class Person {
private String name;
private SequencedAddress address;
private Status status;
private Duration latestLunchBreakDuration = Duration.ZERO;

public Person() {
}
Expand Down Expand Up @@ -64,8 +71,27 @@ public void setStatus(Status status) {
this.status = status;
}

/**
* Need to explicitly set the scale (and the precision so that the scale will actually be read from the annotation).
* Postgresql would only allow maximum scale of 6 for a `interval second`.
*
* @see org.hibernate.type.descriptor.sql.internal.Scale6IntervalSecondDdlType
*/
@Column(precision = 5, scale = 5)
//NOTE: while https://hibernate.atlassian.net/browse/HHH-16591 is open we cannot replace the currently used @JdbcType annotation
// with a @JdbcTypeCode( INTERVAL_SECOND )
@JdbcType(PostgreSQLIntervalSecondJdbcType.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is supposed to be simply @JdbcType(INTERVAL_SECOND)? See https://docs.jboss.org/hibernate/orm/current/userguide/html_single/Hibernate_User_Guide.html#_duration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I thought looking at the docs, too 😃, but the annotation wants a class rather than an int 🙈. Should we update that to be @JdbcType(INTERVAL_SECOND_JDBC_TYPE_CLASS) or something similar?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be @JdbcTypeCode(INTERVAL_SECOND), I believe. And yes it would be nice to send a PR to update the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will test it locally and then send an update to the docs, thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any news on this one? It looks like you didn't update the PR, is it because @JdbcTypeCode didn't work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @JdbcTypeCode lead to https://hibernate.atlassian.net/browse/HHH-16591 🙈
And since someone picked up that ticket to work on, I thought I'd give it a chance to see if it gets fixed 😃
The missing parts are:

Right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding my previous comment.. it's a nice to have but definitely not a requirement ;)

public Duration getLatestLunchBreakDuration() {
return latestLunchBreakDuration;
}

public void setLatestLunchBreakDuration(Duration duration) {
this.latestLunchBreakDuration = duration;
}

public void describeFully(StringBuilder sb) {
sb.append("Person with id=").append(id).append(", name='").append(name).append("', status='").append(status)
.append("', latestLunchBreakDuration='").append(latestLunchBreakDuration)
.append("', address { ");
getAddress().describeFully(sb);
sb.append(" }");
Expand Down