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

IllegalArgumentException when calling UpdatableRecord methods on tables with embedded domains that replace their underlying fields #15873

Closed
NorbertKawinski opened this issue Nov 25, 2023 · 16 comments

Comments

@NorbertKawinski
Copy link

Expected behavior

New record is created in the database

Actual behavior

Exception in thread "main" java.lang.IllegalArgumentException: Field ("author"."id") is not contained in Row (
  "author"."id",
  "author"."name"
)

Steps to reproduce the problem

For such database:

CREATE DOMAIN author_id AS uuid NOT NULL;
CREATE TABLE author
(
    id   author_id    NOT NULL DEFAULT gen_random_uuid() PRIMARY KEY,
    name varchar(255) NOT NULL
);

Generated with
embeddableDomains = ".*"

This code

AuthorRecord author = sql.newRecord(AUTHOR);
author.setName("Sample Author");
author.store();

throws an exception.
Without the embeddableDomains flag, everything works fine.

jOOQ Version

org.jooq.trial:jooq:3.18.7

Database product and version

postgres:16.1-alpine Docker image

Java Version

Openjdk 21.0.1

OS Version

Microsoft Windows 11 Pro [10.0.22621 Build 22621]

JDBC driver name and version (include name if unofficial driver)

org.postgresql:postgresql:42.6.0

@NorbertKawinski
Copy link
Author

NorbertKawinski commented Nov 25, 2023

MCVE available here: https://github.com/NorbertKawinski/jOOQ-mcve

EDIT: Probably related, the embedded keys generation have the same issue.

@lukaseder
Copy link
Member

Thanks a lot for your report. I'll look into this right away.

@lukaseder
Copy link
Member

Yeah, I can reproduce it, with or without defaults.

@lukaseder
Copy link
Member

Stack trace of the new integration test:

java.lang.IllegalArgumentException: Field ("EMBEDDABLE_DOMAINS_IN_KEYS"."ID") is not contained in Row (
  "EMBEDDABLE_DOMAINS_IN_KEYS"."ID",
  "EMBEDDABLE_DOMAINS_IN_KEYS"."V"
)
	at org.jooq.impl.Tools.indexFail(Tools.java:2157)
	at org.jooq.impl.Tools.indexOrFail(Tools.java:2164)
	at org.jooq.impl.AbstractRecord.changed(AbstractRecord.java:574)
	at org.jooq.impl.UpdatableRecordImpl.store0(UpdatableRecordImpl.java:205)
	at org.jooq.impl.UpdatableRecordImpl.lambda$0(UpdatableRecordImpl.java:150)
	at org.jooq.impl.RecordDelegate.operate(RecordDelegate.java:144)
	at org.jooq.impl.UpdatableRecordImpl.store(UpdatableRecordImpl.java:149)
	at org.jooq.impl.UpdatableRecordImpl.store(UpdatableRecordImpl.java:141)
	at org.jooq.codegen.test.embeddabledomains.EmbeddableDomainsTest.testEmbeddableDomainsInKeys(EmbeddableDomainsTest.java:130)

@lukaseder
Copy link
Member

The problem is that the generated Table.getPrimaryKey() is referencing the underlying columns, not the embeddables. I'll investigate separately, if this needs to be changed. Such a change couldn't be backported for compatibility reasons:

However, it's probably possible to fix this even without the above change, and backport the fix.

@lukaseder
Copy link
Member

Within jOOQ, there is an internal UpdatableRecord::getPrimaryKey() method to fetch generated keys:

image

Given that it's internal (package private), we could just fix that particular method for this case here.

@lukaseder
Copy link
Member

As hinted by the call stack, the other UpdatableRecord methods are also likely affected. E.g. refresh():

java.lang.IllegalArgumentException: Field ("EMBEDDABLE_DOMAINS_IN_KEYS"."ID") is not contained in Row (
  "EMBEDDABLE_DOMAINS_IN_KEYS"."ID",
  "EMBEDDABLE_DOMAINS_IN_KEYS"."V"
)
	at org.jooq.impl.Tools.indexFail(Tools.java:2157)
	at org.jooq.impl.AbstractRecord.get(AbstractRecord.java:333)
	at org.jooq.impl.Tools.addCondition(Tools.java:4133)
	at org.jooq.impl.Tools.addConditions(Tools.java:4121)
	at org.jooq.impl.UpdatableRecordImpl.refresh(UpdatableRecordImpl.java:433)
	at org.jooq.impl.UpdatableRecordImpl.refresh(UpdatableRecordImpl.java:425)
	at org.jooq.codegen.test.embeddabledomains.EmbeddableDomainsTest.testEmbeddableDomainsInKeys(EmbeddableDomainsTest.java:137)

@lukaseder lukaseder changed the title IllegalArgumentException when storing record with embeddable domain IllegalArgumentException when calling UpdatableRecord methods on tables with embedded domains Nov 28, 2023
@lukaseder
Copy link
Member

@lukaseder lukaseder changed the title IllegalArgumentException when calling UpdatableRecord methods on tables with embedded domains IllegalArgumentException when calling UpdatableRecord methods on tables with embedded domains that replace their underlying fields Nov 28, 2023
@lukaseder
Copy link
Member

This only happens when the embeddable replaces the underlying column(s)

@lukaseder
Copy link
Member

Duplicate of #12689

@lukaseder
Copy link
Member

Fixed in versions:

Thanks again for your report! I will continue investigating related edge cases (e.g. embeddable keys, keys combining domains and non-embeddables) to see if other, related problems still exist. Will link to further issues from here.

3.19 Other improvements automation moved this from To do to Done Nov 28, 2023
lukaseder added a commit that referenced this issue Nov 28, 2023
…s on tables with embedded domains that replace their underlying fields
lukaseder added a commit that referenced this issue Nov 28, 2023
methods on tables with embedded domains that replace their underlying
fields
lukaseder added a commit that referenced this issue Nov 28, 2023
methods on tables with embedded domains that replace their underlying
fields
lukaseder added a commit that referenced this issue Nov 28, 2023
methods on tables with embedded domains that replace their underlying
fields
@lukaseder
Copy link
Member

I realise there was a comment here:

EDIT: Probably related, the embedded keys generation have the same issue.

What did you mean by this? Enabling both embedded keys and domains? This is currently not working due to the inability to nest embeddables:

Or did you have something else in mind?

@lukaseder
Copy link
Member

@NorbertKawinski
Copy link
Author

NorbertKawinski commented Nov 28, 2023

Ah, no, not that complex with nesting, sorry for not being clear :)
I just meant that the same issue "Field (xxx) is not contained in Row" happened when I used "embeddablePrimaryKeys" --instead of-- domains.

Actually this is how I first bumped into this issue because I started generating the embedded keys.
When it didn't work I tried using domains but got the same issue.

If you'd like to see what I mean, then I updated my MCVE with "embeddablePrimaryKeys"
NorbertKawinski/jOOQ-mcve@e04cc82

@lukaseder
Copy link
Member

Ah, no, not that complex with nesting, sorry for not being clear :)
I just meant that the same issue "Field (xxx) is not contained in Row" happened when I used "embeddablePrimaryKeys" --instead of-- domains.

I see, I'll check that as well, thanks. When combining the two flags (which isn't unlikely), then, nesting would have to occur, though. The domain generates an embeddable first, and then the key generates another one after, based on the first. This doesn't work yet.

@lukaseder
Copy link
Member

I can confirm, this fixes also the case of using embeddablePrimaryKeys (or embeddableUniqueKeys)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants