jakartaee / persistence

https://jakartaee.github.io/persistence/
Other
186 stars 55 forks source link

consider adding JDBC type to @Column or a zoo of built-in AttributeConverters #619

Open gavinking opened 2 months ago

gavinking commented 2 months ago

Currently there are two ways to customize the column type mapped by a field:

  1. using an AttributeConverter, or
  2. using @Column(columnDefinition).

An AttributeConverter works very well for basic types, but there are no built-in AttributeConverters, and it's a pain for the user to have to write one. And it doesn't work at all for associations.

On the other hand columnDefinition doesn't work very well, for several reasons, including that it's not at all portable between databases, and also doesn't do any sort of type conversion at the Java level.

First approach

Since very ancient times, Hibernate allowed the column type to be customized in a portable way by specifying a JDBC type code from the list given in java.sql.Types. Today this is usually done via the @JdbcTypeCode annotation.

The only thing about this approach that has always left me a little uncomfortable is the lack of typesafety, due to the Types being integers. A way to alleviate this discomfort would be to use the JDBCType enumeration instead. So we could add:

public @interface Column {
    ...
    JDBCType type() default JDBCType.OTHER;
}

where we would be abusing OTHER to mean "default".

So I could write:

@Column(type=JDBCType.DECIMAL)
int total;

The spec would need to define which combinations of (field type, JDBC type) an implementation is required to support, but just like with the JPQL cast() function, that list need not be extremely long. That is, it could, at least initially, just be basic numeric conversions and conversions to varchar.

Second approach

Alternatively, if this abuse of OTHER is unacceptable, we could add a new annotation:

public @interface ColumnType {
    JDBCType value();
}

which might anyway be a cleaner approach.

I would write:

@ColumnType(JDBCType.DECIMAL)
int total;

Third approach

Alternatively, the specification could provide a set of built-in attribute converter classes, for use with @Convert. The problem is that to provide converters for n Java types to m JDBC types requires nm classes, which gets a bit messy.

It would look like:

@Convert(converter=Converters.IntegerToDecimal.class)
int total;

which is a bit more verbose than the previous options.

The advantage of this approach is that it requires no new concept in the spec. The major disadvantage is that attribute converters are disallowed for primary and foreign key columns.

gavinking commented 2 months ago

Of course, there's also a

Fourth approach

which is just to do what Hibernate does today and use int-valued codes from Types.

public @interface Column {
    ...
    int type() default Integer.MIN_VALUE;
}

letting me write:

@Column(type=Types.DECIMAL)
int total;

The payoff for the loss of typesafety is that the set of allowed type codes is provider-extensible, which is definitely useful.

beikov commented 2 months ago

I think both, the first and second approach are ok, though the first one seems easier as the second might also require the addition of @JoinColumnType etc.

The fourth option is provider extensible, but I think for provider specifics, it's best to stick to custom annotations.

Also, I don't think that JDBCType is a fitting name, since what is being described here is an abstract SQL type.

gavinking commented 2 months ago

Also, I don't think that JDBCType is a fitting name, since what is being described here is an abstract SQL type.

Well, OK, but it's java.sql.JDBCType and we can't change that (and I would not want to introduce a new enum isomorphic to it either).

beikov commented 2 months ago

Ah, you were referring to the JDK enum. I guess it's ok to reuse that, though that limits JPA to the types JDBC vendors agree on. I think I prefer that we leave the door open to allow extensions that go beyond the JDBC spec.

gavinking commented 2 months ago

I think I prefer that we leave the door open to allow extensions that go beyond the JDBC spec.

Yeah, I suppose it would not be crazy to introduce our own enum, so allow for stuff like UUID.