Thread: Re: [JDBC] JPA + enum == Exception

> -----Original Message-----
> From: pgsql-jdbc-owner@postgresql.org [mailto:pgsql-jdbc-owner@postgresql.org] On Behalf Of Marc G. Fournier
> I'm trying to use enum's in a database, but the java guys are telling me that they are having problems with inserts
...
> reading from the database isn't a problem, but there appears to be an issue with converting from string -> enum when
savingit back again ... 

This is interesting, it seems to be a difference between executing the
sql directly and using a prepared statement:

tomtest=# create type mood as enum ('happy', 'meh', 'sad');
CREATE TYPE
tomtest=# create table enumcast  (current_mood mood);
CREATE TABLE
tomtest=# insert into enumcast values ('sad');
INSERT 0 1
tomtest=# select * from enumcast ;
 current_mood
--------------
 sad
(1 row)


That works ok, but when attempting to use a prepared statement:

    ps = con.prepareStatement("insert into enumcast values (?)");
    ps.setString(1, "meh");
    ps.executeUpdate();

we get a

org.postgresql.util.PSQLException: ERROR: column "current_mood" is of
type mood but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.

Cue sad trombone. You can fix this with implicit casts using CREATE
CAST, or an explicit cast in the query, but this shouldn't really be
necessary for what is a basic use case for enums. In any case ORMs
won't know how to do that without writing custom converters, which
makes me sad. I had intended that ORMs could just treat enum fields as
text fields basically and not have to care about the underlying
implementation.

Cc'ing hackers - why the difference here? I presume that the input
function is getting triggered when the value is inline in the SQL, but
not so when the statement is prepared. Should we consider creating an
implicit cast from text to enums when we create an enum? Or is there
some other way to get the expected behaviour here?

Cheers

Tom

Tom Dunstan <pgsql@tomd.cc> writes:
> ... That works ok, but when attempting to use a prepared statement:

>     ps = con.prepareStatement("insert into enumcast values (?)");
>     ps.setString(1, "meh");
>     ps.executeUpdate();

> we get a

> org.postgresql.util.PSQLException: ERROR: column "current_mood" is of
> type mood but expression is of type character varying
>   Hint: You will need to rewrite or cast the expression.

AFAIK this is just business as usual with JDBC: setString() implies that
the parameter is of a string type.  It'll fall over if the type actually
required is anything but a string.  (I'm no Java expert, but I seem to
recall that using setObject instead is the standard workaround.)

Enums are not suffering any special hardship here, and I'd be against
weakening the type system to give them a special pass.

            regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:

>> when attempting to use a prepared statement:
>
>>     ps = con.prepareStatement("insert into enumcast values (?)");
>>     ps.setString(1, "meh");
>>     ps.executeUpdate();
>
>> we get a
>
>> org.postgresql.util.PSQLException: ERROR: column "current_mood"
>> is of type mood but expression is of type character varying
>>   Hint: You will need to rewrite or cast the expression.
>
> AFAIK this is just business as usual with JDBC: setString() implies that
> the parameter is of a string type.  It'll fall over if the type actually
> required is anything but a string.  (I'm no Java expert, but I seem to
> recall that using setObject instead is the standard workaround.)

Right.  It is spelled out pretty specifically in the JDBC spec:

http://download.oracle.com/otn-pub/jcp/jdbc-4_1-mrel-spec/jdbc4.1-fr-spec.pdf

Table B-2 governs setting PreparedStatement parameters with
setString and similar methods, while tables B-4 and B-5 cover the
setObject methods.  It is clearly not unusual for other vendors to
extend the JDBC specification to make life easier for those writing
ORMs, etc.; but the behavior of the current PostgreSQL JDBC driver
is doing all that is required by the spec.

> Enums are not suffering any special hardship here, and I'd be against
> weakening the type system to give them a special pass.

This is not entirely unrelated to the discussions about allowing
broader use of automatic casting server-side.  It seems to me that
on one side of the argument is the idea that strict typing reduces
bugs and doesn't lead to problems with ambiguity, especially as
things change; and on the other side the argument is that where no
ambiguity exists we would make life easier for developers of
applications or access tools if we relexed things beyond what the
related specifications require, and that not doing so discourages
adoption.  I think that all the same arguments apply here with
equal force, on both sides of the issue.

The problem with this debate has always been that both sides are
completely right.  Those are always the toughest to resolve.  It
comes down to which evils we tolerate to garner which benefits.  It
seems that in such cases inertia tends to win.  I'm not so sure
that it should.  An ideal solution would find some way to address
the concerns of both sides, but so far that has eluded us when it
comes to the type system.

-Kevin

AFAIK this is just business as usual with JDBC: setString() implies that
the parameter is of a string type.

Well, it means that it's a type compatible with a java.lang.String . JDBC doesn't say much about the database-side type.

It's my strong view that we should accept setString(...) for any string-like type, like xml, json, domains of text, and so on - or at least provide an easy, no-superuser-required way to tell Pg to do so.

I've pushed a testcase to:

    https://github.com/ringerc/scrapcode/tree/master/testcases/postgresql/jdbc

that you can run if you want to see/experiment with the JDBC behaviour. I'll attach a tgz of it to a follow-up mail.

Notably, you'll see that setObject(position, "value") is insufficient; you need to explicitly invoke setObject with Types.OTHER.

  It'll fall over if the type actually
required is anything but a string.  (I'm no Java expert, but I seem to
recall that using setObject instead is the standard workaround.)

Unfortunately, setObject(...) isn't a complete workaround; it doesn't know about PostgreSQL-specific types, and setObject(1, "value") won't work, you have to explicitly specify Types.OTHER, eg setObject(1, "{\"key\" : \"value\"}", Types.OTHER);

More importantly, for many Java users there are layers on top of JDBC that expect the database to be moderately lenient about accepting java.lang.String arguments. The extreme strictness Pg imposes makes things hard because most of these tools don't provide ways to punch through the abstraction and specify database types - they haven't needed to, because other DBs let you implicitly cast to/from things that look like strings.

PgJDBC could work around this by treating setString(...) and the two-argument setObject(...) as if they were setObject(..., Types.Other). However, this would risk breaking queries that currently rely on the explicit text type to resolve functions that would otherwise be ambiguous. The same issue may apply to making conversions for text-like types more lenient in the server, and it wouldn't help people who want to:

@Entity
public class MyJPAEntity {
    //....

    @Column
    private String jsonField;

}

... in their JPA mappings backed by Hibernate/EclipseLink/etc. They really need a session-level, DB-level or user-level (ie: GUC) way to say "Let me cast implicitly between json/xml/etc and text".

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

Craig Ringer <craig@2ndquadrant.com> writes:
> On 02/08/2013 12:55 PM, Tom Lane wrote:
>> AFAIK this is just business as usual with JDBC: setString() implies that
>> the parameter is of a string type.

> Well, it means that it's a type compatible with a java.lang.String .
> JDBC doesn't say much about the database-side type.

> It's my strong view that we should accept setString(...) for any
> string-like type, like xml, json, domains of text, and so on - or at
> least provide an easy, no-superuser-required way to tell Pg to do so.

The difficulty I've got with that is that there are only two kinds of
literal in SQL, namely numbers and strings, so that "it looks like a
string" applies to absolutely every non-numeric type.  If we go down
this road we'll end up allowing implicit casts from text to (at least)
every non-numeric type, which will be pretty much fatal for type
checking purposes.

IIRC, there's already a hack to tell the JDBC driver to mark setString
parameters as "unknown" rather than "text", which seems to me to be
a much less dangerous way of getting the lenient behavior when you want
it.

            regards, tom lane

On 02/12/2013 11:50 PM, Tom Lane wrote:
> Craig Ringer <craig@2ndquadrant.com> writes:
>
>> It's my strong view that we should accept setString(...) for any
>> string-like type, like xml, json, domains of text, and so on - or at
>> least provide an easy, no-superuser-required way to tell Pg to do so.
>
> The difficulty I've got with that is that there are only two kinds of
> literal in SQL, namely numbers and strings, so that "it looks like a
> string" applies to absolutely every non-numeric type.

I absolutely agree with that part - we don't want to go converting
*everything* from 'text' to whatever we feel like implicitly. The
removal of those implicit casts was by and large a good thing.

I'm focusing specifically on data types that make sense to handle as
strings in client applications - types where there may be no universal,
core, built-in data type for them or where handling them as strings in
the client may be dramatically more efficient - like "json" and "xml".

I'm *not* proposing a blanket implicit conversion. Those conversions
were removed for good reasons. I just think the removal went a little
too far and that a couple of them need to be added back in. I'm *not*
arguing that we should implicitly convert "text" to anything and everything.

Here's the rationale:

Because of the growing trend toward declarative, type-based data
mapping, it is no longer easy for many users to separate low-level
database interaction from the higher level definitions of how the data
model is represented and worked with in the application. Irrespective of
whether these high level data mapping tools are a good thing or not, in
practice the effect is that it's not easy to say "I'll store this JSON
as a string in my app, and tell the DB it's an unknown type literal when
doing database I/O with it so it knows it can cast it to its internal
JSON type". You largely lose access to the JDBC layer, and while most
JPA implementations and other tools offer ways to pierce the abstraction
they can be clumsy, hard to find out about, poorly documented, and
difficult. That's OK if you're doing something weird and special - but I
don't think storing and retrieving json and xml values as strings rather
than rich data types in the client falls into that category.

Yes, I'm saying we should work around client issues where it has a low
cost to us.

We could say "Use a client that doesn't suck". That's fine; we're not
selling database systems, so when the user says "no thanks, I'll use a
database that doesn't suck instead" it doesn't directly hurt us.
However, I'd prefer not to force that choice when we can fix the problem
on our end with no negative impact on ourselves or users.

I *really* don't want users to have to use memory- and cpu-hungry types
from some XML or JSON support library when mapping database entities
into the application model just to work around a type handling quirk
from the interaction of the client library, Pg and PgJDBC. Particularly
when the app (or this layer of it) in question might be just an
intermediary that doesn't really care what's in the textlike field.

We work around less-than-lovely quirks in operating systems, the SQL
standard, other DBs, etc all the time. I'm proposing that we work around
one in widely used clients like Hibernate and EclipseLink since it
doesn't hurt us and it makes users' lives easier.

> If we go down
> this road we'll end up allowing implicit casts from text to (at least)
> every non-numeric type, which will be pretty much fatal for type
> checking purposes.

I see what you're getting at and agree that this would be bad, but it's
not what I'm arguing for.

I specifically think that "json" and "xml" should be implicitly castable
to/from text. Permitting this doesn't require changes to how Pg
interprets literals, nor hacks in the JDBC driver.

Hacking the JDBC driver to send all java.lang.String values as unknown
is exactly what I want to avoid.

I'd just tell people to:

  CREATE CAST (text AS xml) WITH FUNCTION xml(text) AS IMPLICIT;
  CREATE CAST (text AS json) WITHOUT FUNCTION AS IMPLICIT;

but (a) the "xml" one won't work because a cast already exists; (b) it's
superuser-only; and (c) it relies on implementation details that may change.

In practice you need to, as superuser:

  UPDATE pg_catalog.pg_cast
  SET castcontext = 'i'
  WHERE casttarget = 'xml'::regtype
  AND (castsource = 'text'::regtype OR castsource = 'varchar'::regtype);

then query pg_cast to see if a text to json cast already exists, update
it if it does exist, and use the above CREATE CAST to create it if it
doesn't.

In other words, "blech".

> IIRC, there's already a hack to tell the JDBC driver to mark setString
> parameters as "unknown" rather than "text", which seems to me to be
> a much less dangerous way of getting the lenient behavior when you want
> it.

I'll take a look. I never noticed one in the docs, but back when I was
wrestling with this problem I don't think I dug into the PgJDBC sources.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services