Jooq: Add support for PostgreSQL ON CONFLICT .. WHERE <index_predicate>

[lukaseder]

We already support ON CONFLICT .. DO UPDATE .. WHERE as of #5637, but there's another possible WHERE clause that further specifies the ON CONFLICT clause:

where conflict_target can be one of:

( { index_column_name | ( index_expression ) } 
      [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ]

---

See also:
https://www.postgresql.org/docs/current/static/sql-insert.html

[timur-sh]

There is missed there ON CONSTRAINT constraint_name clause. It provides useful functional allowing to specify constraint by name. That's why it should be added.

[lukaseder]

Thanks for your message, @timur-sh. I've created a separate feature request for this: #6906

[dzagorovsky]

@lukaseder do you have any plans to support PostgreSQL ON CONFLICT ... WHERE ... DO UPDTAE?
Maybe there is some non-plain sql workaround?

[lukaseder]

Yes, the plan is this issue. No, there's no non-plain SQL workaround.

[DaanVandenBosch]

As far as I know, the only workaround at the moment is reverting to dslContext.query("sql string"). Is this correct?

And @lukaseder, I'd like to thank you and your team for building and open sourcing such a wonderful tool! I'm so glad I've finally found a more powerful alternative to an ORM. Working with JOOQ code is so much more pleasant than a tangle of Spring Data, Hibernate Specifications, JPQL, HQL and the unavoidable raw SQL or dropping down to JDBC. I appreciate the Kotlin/Scala support too (or at least it plays nice with other languages).

[lukaseder]

Thanks for your nice words, @DaanVandenBosch.

As far as I know, the only workaround at the moment is reverting to dslContext.query("sql string"). Is this correct?

This is one workaround. Another is to patch the generated SQL with an ExecuteListener. A third is, of course, to patch the library itself :-)

[DaanVandenBosch]

I wouldn't really call the last one a workaround, but I took a shot at it anyway. So far I've only added a InsertOnConflictWhereDoUpdateStep and implemented an onConflictWhere method for a single Condition.
Two problems:

• The method should be named "where", but that clashes with the other where methods in InsertQueryImpl, I'd probably have to use two subclasses to make it work correctly (right now there is no way to know from which step you're calling where)
• It only seems to work when I use inlined values in the condition

Do you have any pointers and does my approach make sense?

[lukaseder]

@DaanVandenBosch: Thank you very much for doing this! I'll comment directly on your commit. Regarding your two problems:

The method should be named "where", but that clashes with the other where methods in InsertQueryImpl, I'd probably have to use two subclasses to make it work correctly (right now there is no way to know from which step you're calling where)

Have a look at SelectImpl. It contains several similar cases with methods of "overloaded" semantics, e.g. and(), or(). I'm sure this approach can be used for InsertImpl's where() method calls as well.

It only seems to work when I use inlined values in the condition

Why?

[lukaseder]

Comments here:

• https://github.com/DaanVandenBosch/jOOQ/commit/5ea071fc639c1db3f37fef480528aca977175701#r30237022
• https://github.com/DaanVandenBosch/jOOQ/commit/5ea071fc639c1db3f37fef480528aca977175701#r30236986

[lukaseder]

It only seems to work when I use inlined values in the condition

I think the reason can be derived from the documentation:

index_predicate
Used to allow inference of partial unique indexes. Any indexes that satisfy the predicate (which need not actually be partial indexes) can be inferred. Follows CREATE INDEX format. SELECT privilege on any column appearing within index_predicate is required.

It appears this additional WHERE clause has to use inline values because otherwise, the appropriate partial index couldn't be chosen by PostgreSQL. That seems to be more of an implementation detail leaked into the language...

So the solution here would be to wrap binding the value by a Context.paramType() call.

[DaanVandenBosch]

Thanks for your comments, Lukas. I'll probably continue working on it tomorrow, I'll fix the naming issues, add the paramType call and add more convenience methods, etc.

I also took a quick look at SelectImpl, and I'm glad I did. I didn't know query objects were stateful w.r.t. to the current query step. I remember the documentation saying that you shouldn't use them to build queries dynamically, now I know why!

I feel like each step returning a different subclass (when necessary) that delegates to the main SelectImpl (or InsertImpl) would solve the problem and would allow dynamic query building. Or does this introduce more problems than it solves?

Right now, I'll use the current method from SelectImpl since this is not the ticket to tackle this issue. But I think people intuitively want to use the query objects dynamically (at least I've done it, I wouldn't even think to look this up in the docs first and it seems to work mostly), so I'm very curious what led to this design.

[lukaseder]

I feel like each step returning a different subclass (when necessary) that delegates to the main SelectImpl (or InsertImpl) would solve the problem and would allow dynamic query building. Or does this introduce more problems than it solves?

Sure, but let's solve one problem at a time. jOOQ 4.0 will be about immutability. This is a very big project.

Right now, I'll use the current method from SelectImpl since this is not the ticket to tackle this issue.

Yes

But I think people intuitively want to use the query objects dynamically (at least I've done it, I wouldn't even think to look this up in the docs first and it seems to work mostly)

People have also assigned step types to local variables and kept calling the same methods upon them... There's pros and cons to each approach. Of course, more cons to the status quo in the long run, because neither intermediate steps nor complete queries are thread safe.

Some details here:
https://www.jooq.org/doc/latest/manual/sql-building/sql-statements/dsl-and-non-dsl/

so I'm very curious what led to this design.

Unwise decisions in early jOOQ and a big dedication to backwards compatibility. Again, this needs to be fixed thoroughly in a large change, so this isn't going to happen in 3.x

[DaanVandenBosch]

Ok, thanks for the explanation. Is there a roadmap or idea bucket for jOOQ 4? You've made me curious now.

And I went ahead and implemented your suggestions right now. It works for my simple queries, but I have no idea about more complex ones. Are there automated tests?

The doc strings are a bit terse at the moment. I'm not a database wizard and I don't want to confuse people, could you give me some advice for documentation? Other comments are welcome too. Thanks for responding so quickly and allowing me to get this done so fast.

Edit: I've only tested with PG 10.1 with ON CONFLICT clauses like:

.onConflict(TASK_ACTIVE.CONCURRENCY_KEY)
.where(TASK_ACTIVE.STATE.in(EXECUTING, WAITING_FOR_RETRY, WAITING_FOR_CHILDREN))

[lukaseder]

Is there a roadmap or idea bucket for jOOQ 4? You've made me curious now.

Of course: https://github.com/jOOQ/jOOQ/milestone/78

Are there automated tests?

Of course, but not open source. I can run the tests for you if you would like to send a PR. Please also make sure to sign the transfer of rights agreement, such that your contribution can be dual licensed:
https://github.com/jOOQ/jOOQ/blob/master/CONTRIBUTING.md

The doc strings are a bit terse at the moment. I'm not a database wizard and I don't want to confuse people, could you give me some advice for documentation? Other comments are welcome too

It's easier for me to fix these things directly rather than giving hints about how they could be fixed.

[DaanVandenBosch]

I figured I should probably mention it here too for anyone who's watching this issue, but there's a pull request here: #7807.

[lukaseder]

@DaanVandenBosch Thank you very much. I've seen it, of course. I'll look into it next week. Thank you very much for your patience

[Nayshins]

Any updates on this feature being released?

[connec]

I've bumped into this limitation as well.

Another possible workaround I'd hoped to use is to simply not specify the constraint, e.g using .onConflictDoNothing() which generates ON CONFLICT DO NOTHING in the SQL.

Unfortunately there isn't an equivalent for DO UPDATE (e.g. .onConflictDoUpdate()), and any attempt to leave the constraint unspecified (e.g. onConflict().doUpdate() or onDuplicateKeyUpdate()) implicitly generates ON CONFLICT ("id") DO UPDATE.

[lukaseder]

Thanks for your messages, folks. I will implement this now.

[Nayshins]

Thanks for getting to this @lukaseder. Do you have an idea when this will be released?

[lukaseder]

Do you have an idea when this will be released?

Yes, it will be released

[mkurz]

@lukaseder the question from @Nayshins was when this will be released.

[lukaseder]

@mkurz I know 😉.

We'll release 3.14 "soon," but I don't want to give any promises, which are never kept, and especially not for a very minor feature like this.

[lukaseder]

To give you some context. The two main features of 3.14 are:

• JSON/XML support
• Much better Kotlin support

There is still a lot to polish in these areas, especially the Kotlin support. If the release is rushed, then the published API will be set in stone. Changing it will not be possible without breaking things. That's very undesirable.

In that context, this <index_where> predicate doesn't seem like a feature whose availability should have any impact on the release 3.14.

[Nayshins]

Thanks for the background, we have a feature that is blocked without the index_where, but I will go with a workaround for now. Thanks @lukaseder for your responsiveness!

[Captain1653]

Hi @lukaseder .
Thank you for the feature, but I have some strange behavior. Can you help me? Let me explain:

1. I have some table

create table course_enroll
(
    id bigint not null
        constraint course_enroll_pk
            primary key,
    token text not null,
    user_id bigint not null,
    course_id bigint not null,
    attempts_limit integer default 1,
    attempts_used integer,
    deleted_ts timestamp,
    ac_host_id bigint,
    created_ts timestamp,
    updated_ts timestamp
);

create unique index "course-enroll-one-active-uidx"
    on course_enroll (user_id, course_id)
    where (deleted_ts IS NULL);

1. I have some jooq insert-code and onConflict:

multiUpsert.onConflict(COURSE_ENROLL.USER_ID, COURSE_ENROLL.COURSE_ID)
                .where(COURSE_ENROLL.DELETED_TS.isNull())
                .doUpdate()
                .set(COURSE_ENROLL.UPDATED_TS, now())
                .returning(COURSE_ENROLL.fields());

1. But the onConflict generated not valid SQL:

insert into "public"."course_enroll" ("id", "token", "user_id", "course_id", "attempts_limit", "attempts_used",
                                      "deleted_ts", "ac_host_id", "created_ts", "updated_ts")
values (nextval('"public"."course_enroll_seq"'), 'E6jFiIyC2NClZpMmj-87X0u9Zft7tNNtlhQ6kZ3g8N0FFLtU3uDfleJUOqq', 1,
        1, 1, 0, null, 50, timestamp '2021-03-19 13:39:25.62', timestamp '2021-03-19 13:39:25.62')
on conflict ("user_id", "course_id")
where "public"."course_enroll"."deleted_ts" is null do   
update -- emphasis here
set "updated_ts" = timestamp '2021-03-19 13:39:25.62';

my IDE show me error:

[42P01] ERROR: invalid reference to FROM-clause entry for table "course_enroll" There is an entry for table "course_enroll", but it cannot be referenced from this part of the query. Позиция: 469

It works for where deleted_ts is null do update (just use simple name deleted_ts).

Did I make some mistake? Is it really important to use simple name? Can you help me to generate the correct SQL-query?
I use postgres 12.5 and liquibase, jooq 3.14.8. Please, let me know if you have any questions.

Kind regards

[lukaseder]

Thanks for your report. Might be a bug. Can you please open a new issue?