Add upsert and/or fetchByKey(Class, Object...) methods to Session

[dstango]

I'm just exploring this library on Informix and it's basically working though it doesn't have "official" support - so that good :-).

What I'm missing is a kind of upsert-statement: if an object is alread in the database I'd like to update it, if it's not there I'd like to insert it, e.g.:

try (Session session = new Session(dataSource.getConnection())) {
    session.upsert(customer);			
}

I tried to work around it by using fetch first and then deciding if I need to insert or update. But I couldn't find a method which I could pass in the requested class and the key, e.g.:
Customer customer = session.fetch(Customer.class, 5);
Actually -- I found exactly that method, but it's commented out, as it doesn't work together with the variant that accepts an sql as a second parameter)... so that's a method that I'm missing, too.

There is a fetch-method with a single POJO parameter, but it needs to be filled with the primary key first and when querying it, it will overwrite all contents contained in it. I could work around it by cloning the object, but that would require the POJO to be Cloneable, and it doesn't feel very right conceptually:

    Customer customerThrowAway = customer.clone();
    if (session.fetch(customerThrowAway)) {
        session.update(customer);
    } else {
        session.insert(customer);
    }

So I'm kind of stuck here. I can of course write a simple SELECT myself and use the fetch-method with the SQL as a second parameter, but that doesn't really fit into the concept of simplicity with zero ceremony ;-) ...

So I'd propose

• to "wake up" the commented out fetch-method, probably with a new name
• (an alternative to using a different name would be to change the existing fetch(Class, String, Object...) signature to fetch(String, Class, Object...) -- it should avoid the clash of signatures, but would break backwards compatibility).
• consider adding an upsert-method

So what do you think? Thanks for consdidering :-)

[Background: My usage szenario is that I'm offering a REST webservice where the consumer can request a PUT some special field value. This value is usually just some optional add-one to some other POJO. In the GET-request we simply provide a default value if there's no value available, but we need to offer the option to store something explicitly.]

[sproket]

The simple fetch you see commented out is because the overload collides if you have a primary key that is a string. The usual method is fetch(class, sql, params) so you can just use fetch(class, "select * from table where id = ?", 123)

Or if you know the primary key value to find it

Customer customer = new Customer();
customer.setId(123);

if (session.fetch(customer)) {
update();
} else {
insert();
}

But you're right. I could have an "upsert" method.

Yeah I've been looking at that fetch method. I was thinking of writing a Parameter class to capture the parameters as a type. Then later add named parameters as a feature. I know it would break the existing API but it might be OK in a 1.1 or 2.0 release to do that.

Also I'm adding a withTransaction() which simplifies transactions and getting records to work. ;)

In terms of writing the SQL yourself. I consider that to be feature, not a bug. I think most ORMs fail on trying too hard to abstract away SQL.

Good ideas! If I have time this weekend I'll work on getting Informix added to the test suite.

[dstango]

Thanks for the quick response!

[I definitely like to have the ability to write the SQL myself -- actually that's why I'm looking into alternatives to JPA like your library. We've implemented a webservice to abstract away various tables from different databases (and even other systems), but so far only for read (GET) requests. We use dbutils so far and some homebrewn framework to use some SQL-templates and "fill in" dynamically some additions to the WHERE clause, depending on the requests needs, and that has been working extremely well. (We have a parallel "sibling"-project with another team doing something similar, but using mostly JPA, and so far I like the plain-SQL-approach much better than fighting with the Criteria API or JPQL ...)]

Anyhow: I'm looking for something generic regarding the CUD-part of CRUD now, as we need to write some things back into the database. And I'd very much like to have the SQL generated automatically for these cases ...

I'll consider your proposal, but it requires me for my implementation of an upsert to not be able to write it generically for any POJO -- that's what's concerns me, as I'd like to write it just once.

Another idea: Maybe we can better live without the fetch with parameters if there would be an exists-method, that doesn't touch the POJO, but merely checks if a record is in the database, e.g. like this:

if (session.exists(customer)) {
    update(customer);
} else {
    insert(customer);
}

Otherwise I need to know about all the details of the customer, the id, the table name etc. -- and that's actually all handled by Persism already for insert/update anyhow ... so why would I bother to do that manually?!? ;-)

Actually implementing a generic upsert in Persism might benefit from some transaction handling to prevent race conditions, so I'm curious what you're up to.

[Informix is working good so far for my simple test cases, so I'm already satisfied with that ;-) ...]

[dstango]

I found a generic way to implement my upsert:

if (session.update(dto) == 0) {
  session.insert(dto);
}

This could be added as an out-of-the-box solution, though I'm happy with it in my use case.

And I'd still enjoy the exists()-method anyhow ...

[sproket]

FYI - The update would usually always return 1 unless you implement Persistable or extend PersistableObject since these do the check for changed fields.

[dstango]

Can't confirm it's always reporting '1' -- it rather reports the number of affected rows, as JDBC does. I"m not implementing Persistable yet.
But it's a good point to take care of: what happens when the optimizations of Persistable kick in? Maybe my logic for upsert doesn't hold then any more ...

[sproket]

I would recommend for now to use if (fetch()) which returns true if found. Or use the other one and check for null.

[dstango]

Right now I'd looking for a generic way without needing to write individual SQLs for it, and as fetch() overwrites the original content, I'm not happy with that. It all calls for an exists()-method ;-) ....

I'll stick with my current solution for now and add a check for Persistable, as that might result in returning 0 if no fields are modified.

[sproket]

Check out my experimental branch. It's a new API. Let me know what you think.

[dstango]

So regarding this issue we're mainly talking about this method, right? (I also found some more query-methods and overloaded variations of fetch)

public <T> T fetch(Class<T> objectClass, Parameters parameters)

I appreciate the automatic SQL-generation involved which then gets filled with the primary-key-parameters.

I've been thinking about it a little and looked into the source code. If there's only one primary key column this will be fine.
But if we have a composite primary key, there's an issue, that might bite back: the order of the parameters depends on the order of the primary key columns as stored in Persism's metadata. What I could see from the source code is that this order seems to be determined by the order of the columns in the database table. This might differ from the order of the fields in the POJO-class. (Funny side note: DatabaseMetaData.getPrimaryKeys() specifies returning primary key columns sorted by COLUMN_NAME ...)

While this doesn't need to be a problem, there are two concerns:

• It needs to be documented, as it's not obvious.
• It will make an application program break if the order of the pk-columns in the database changes while all the rest remains the same
• It's a candidate to introduce brittleness, as it breaks encapsulation by exposing the internal implementation detail (order of columns) via the interface of fetch. If the internal order of columns changes (which is only provided by the LinkedHashMap, but not by the used interface Map), it will break the interface of fetch.

Thinking about it I start to appreciate the original fetch-method that gets a POJO in a QBE-style, as the order of the columns isn't relevant there.

Here are some options that come to mind:

• specify the required order of the pk columns explicitly in the documentation (what would a programmer expect? Order of the table fields or rather order of the fields in the POJO-class?)
• supply parameters in a key-value-style (e.g. map) which makes the order irrelevant -- unfortunately it makes the method signature somewhat more clumsy (though Map.of() for Java 9+ helps).
• provide the pks in form of a POJO-QBE-object (as before), but return a new instance.

What do you think about it?

Thanks for putting efforts into this issue!

[sproket]

NON standard sql.

[dstango]

NON standard sql.

Don't know how this feature request is dependent on an Sql upsert command.
There are sql dialects with upsert of course, but this is not meant here.

[sproket]

Hi, hope you're doing well!

Yeah MySQL for example has REPLACE.