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

Support overlapping embeddables when enabling <embeddablePrimaryKeys/> #15828

Closed
alf opened this issue Nov 13, 2023 · 10 comments
Closed

Support overlapping embeddables when enabling <embeddablePrimaryKeys/> #15828

alf opened this issue Nov 13, 2023 · 10 comments

Comments

@alf
Copy link

alf commented Nov 13, 2023

Expected behavior

No response

Actual behavior

Lots of [WARNING] Embeddable ambiguity : There are multiple embeddables like the following (line broken down for readability):

[WARNING] Embeddable ambiguity     : There are multiple embeddables ([
    EMNE__PK (referenced by EMNE [
        EMNE__PK.INSTITUSJONSNR_EIER (referenced by EMNE.INSTITUSJONSNR_EIER),
        EMNE__PK.INSTITUSJONSNR (referenced by EMNE.INSTITUSJONSNR),
        EMNE__PK.EMNEKODE (referenced by EMNE.EMNEKODE),
        EMNE__PK.VERSJONSKODE (referenced by EMNE.VERSJONSKODE)
    ]),
    INSTITUSJON__PK (referenced by EMNE [
        INSTITUSJON__PK.INSTITUSJONSNR (referenced by EMNE.INSTITUSJONSNR)])]) matching column list [
            EMNE.INSTITUSJONSNR
    ]

While the tables in question are quite large, I believe the important part here is that:

KJERNEAPI@XEPDB1 SQL> select constraint_name, column_name 
  2                    from user_cons_columns 
  3                   where constraint_name like 'EMNE\_\_PK' ESCAPE '\' 
  4                      or constraint_name like 'EMNE\_\_%INSTITUSJON\_\_FK' ESCAPE '\' 
  5*                  order by constraint_name, position

CONSTRAINT_NAME                     COLUMN_NAME                 
___________________________________ ___________________________ 
EMNE__HAR__INSTITUSJON__FK          INSTITUSJONSNR              
EMNE__PK                            INSTITUSJONSNR_EIER         
EMNE__PK                            INSTITUSJONSNR              
EMNE__PK                            EMNEKODE                    
EMNE__PK                            VERSJONSKODE                
EMNE__SAMARBEID__INSTITUSJON__FK    INSTITUSJONSNR_SAMARBEID    

6 rows selected. 

Am I missing something here?

The tables in question are views and the constraints are virtual in case that matters.

Steps to reproduce the problem

The problem appears when enabling the following setting under database, as per https://www.jooq.org/doc/latest/manual/code-generation/codegen-embeddable-types/codegen-embedded-keys/

<embeddablePrimaryKeys>.*</embeddablePrimaryKeys>

jOOQ Version

3.18.6

Database product and version

Oracle 18 XE

Java Version

No response

OS Version

No response

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

No response

@lukaseder
Copy link
Member

Thanks for your report.

Can you provide a minimal reproducer for this warning (minimal DDL and code generation configuration)? I'm not sure, otherwise, if this warning is legitimate or not. Note, our MCVE template now also has an Oracle version: https://github.com/jOOQ/jOOQ-mcve, though perhaps, just DDL and codegen config will suffice in this case.

alf pushed a commit to alf/jOOQ-mcve that referenced this issue Nov 15, 2023
@alf
Copy link
Author

alf commented Nov 16, 2023

I created an MVCE and pushed to Github. The problem is trivially reproducible. The following schema causes the warning to be logged when you turn on embeddablePrimaryKeys:

CREATE TABLE "INSTITUSJON"
  (
    "INSTITUSJONSNR" NUMBER(8,0) NOT NULL ENABLE,
    CONSTRAINT "I01_INSTITUSJON" PRIMARY KEY ("INSTITUSJONSNR")
  );

CREATE TABLE "EMNE"
  (
    "INSTITUSJONSNR_EIER" NUMBER(8,0) NOT NULL ENABLE,
    "INSTITUSJONSNR" NUMBER(8,0) NOT NULL ENABLE,
    "EMNEKODE" VARCHAR2(12 CHAR) NOT NULL ENABLE,
    "VERSJONSKODE" VARCHAR2(3 CHAR) NOT NULL ENABLE,
    "EMNENAVNFORK" VARCHAR2(20 CHAR) NOT NULL ENABLE,
    "EMNENAVN_BOKMAL" VARCHAR2(120 CHAR) NOT NULL ENABLE,
    CONSTRAINT "I01_EMNE" PRIMARY KEY ("INSTITUSJONSNR_EIER", "INSTITUSJONSNR", "EMNEKODE", "VERSJONSKODE"),
    CONSTRAINT "INSTITUSJON_EMNE" FOREIGN KEY ("INSTITUSJONSNR")
    REFERENCES "INSTITUSJON" ("INSTITUSJONSNR") DEFERRABLE ENABLE
  );

@lukaseder
Copy link
Member

Thanks a lot! I will look at it soon.

@lukaseder
Copy link
Member

I can reproduce this warning:

[WARNING] Embeddable ambiguity : There are multiple embeddables ([MCVE.I01_EMNE (referenced by MCVE.EMNE [MCVE.I01_EMNE.INSTITUSJONSNR_EIER (referenced by MCVE.EMNE.INSTITUSJONSNR_EIER), MCVE.I01_EMNE.INSTITUSJONSNR (referenced by MCVE.EMNE.INSTITUSJONSNR), MCVE.I01_EMNE.EMNEKODE (referenced by MCVE.EMNE.EMNEKODE), MCVE.I01_EMNE.VERSJONSKODE (referenced by MCVE.EMNE.VERSJONSKODE)]), MCVE.I01_INSTITUSJON (referenced by MCVE.EMNE [MCVE.I01_INSTITUSJON.INSTITUSJONSNR (referenced by MCVE.EMNE.INSTITUSJONSNR)])]) matching column list [MCVE.EMNE.INSTITUSJONSNR]

And this compilation error:

[ERROR] /C:/Users/lukas/jOOQ/mcve/15828/jOOQ-mcve/jOOQ-mcve-java-oracle/src/main/java/org/jooq/mcve/java/oracle/Keys.java:[38,94] no suitable method found for createForeignKey(org.jooq.mcve.java.oracle.tables.Emne,org.jooq.Name,org.jooq.TableField<org.jooq.mcve.java.oracle.tables.records.EmneRecord,org.jooq.mcve.java.oracle.embeddables.records.I01EmneRecord>,org.jooq.UniqueKey<org.jooq.mcve.java.oracle.tables.records.InstitusjonRecord>,org.jooq.TableField<org.jooq.mcve.java.oracle.tables.records.InstitusjonRecord,org.jooq.mcve.java.oracle.embeddables.records.I01InstitusjonRecord>,boolean)
    method org.jooq.impl.Internal.<R,U>createForeignKey(org.jooq.UniqueKey<U>,org.jooq.Table<R>,org.jooq.TableField<R,?>...) is not applicable
      (cannot infer type-variable(s) R,U
        (argument mismatch; org.jooq.mcve.java.oracle.tables.Emne cannot be converted to org.jooq.UniqueKey<U>))
    method org.jooq.impl.Internal.<R,U>createForeignKey(org.jooq.Table<R>,org.jooq.Name,org.jooq.TableField<R,?>[],org.jooq.UniqueKey<U>,org.jooq.TableField<U,?>[],boolean) is not applicable
      (cannot infer type-variable(s) R,U
        (argument mismatch; org.jooq.TableField<org.jooq.mcve.java.oracle.tables.records.EmneRecord,org.jooq.mcve.java.oracle.embeddables.records.I01EmneRecord> cannot be converted to org.jooq.TableField<R,?>[]))
    method org.jooq.impl.Internal.<R,U,ER>createForeignKey(org.jooq.Table<R>,org.jooq.Name,org.jooq.TableField<R,ER>,org.jooq.UniqueKey<U>,org.jooq.TableField<U,ER>,boolean) is not applicable
      (inference variable ER has incompatible equality constraints org.jooq.mcve.java.oracle.embeddables.records.I01InstitusjonRecord,org.jooq.mcve.java.oracle.embeddables.records.I01EmneRecord)
    method org.jooq.impl.Internal.<R,U>createForeignKey(org.jooq.UniqueKey<U>,org.jooq.Table<R>,java.lang.String,org.jooq.TableField<R,?>...) is not applicable
      (cannot infer type-variable(s) R,U
        (argument mismatch; org.jooq.mcve.java.oracle.tables.Emne cannot be converted to org.jooq.UniqueKey<U>))
    method org.jooq.impl.Internal.<R,U>createForeignKey(org.jooq.UniqueKey<U>,org.jooq.Table<R>,java.lang.String,org.jooq.TableField<R,?>[],boolean) is not applicable
      (cannot infer type-variable(s) R,U
        (actual and formal argument lists differ in length))

@lukaseder
Copy link
Member

This looks like one of the known issues from here:

Which reads:

Overlapping embeddables should be able to nest, optionally. E.g. a UK on (A, B, C) and an FK on (A, B) should produce an embeddable type of the structural form ((A, B), C)

There are no tests for this scenario yet. Two things need to be done here:

  1. Compilation errors must be fixed
  2. Implementation of the feature

Perhaps, the warning could be improved to make it clear that this isn't supported (yet), in case it's hard to implement correctly.

@lukaseder lukaseder changed the title Multiple warnings about embeddable ambiguity when enabling embeddablePrimaryKeys Support nested embeddables when enabling <embeddablePrimaryKeys/> Nov 16, 2023
@lukaseder lukaseder changed the title Support nested embeddables when enabling <embeddablePrimaryKeys/> Support overlapping embeddables when enabling <embeddablePrimaryKeys/> Nov 16, 2023
@lukaseder
Copy link
Member

It's not strictly the same as the improvement requested in #11975. We already support overlapping embeddables (without nesting them), e.g. (A, B, C) and (B, C, D) is possible, even for pairs of keys. Just not for this particular configuration, so this is simply a bug, which should be fixable.

@lukaseder
Copy link
Member

Hmm, weird. There already is an integration test:

CREATE TABLE s_composite_key.t_composite_key_3 (
  i BIGINT NOT NULL,
  v INT NOT NULL,

  CONSTRAINT pk_t_composite_key_3 PRIMARY KEY (i)
);

CREATE TABLE s_composite_key.t_composite_key_4 (
  j BIGINT NOT NULL,
  k BIGINT NOT NULL,
  v INT NOT NULL,
  
  CONSTRAINT pk_t_composite_key_4 PRIMARY KEY (j, k)
);

CREATE TABLE s_composite_key.t_composite_key_5 (
  i BIGINT NOT NULL,
  j BIGINT NOT NULL,
  k BIGINT NOT NULL,
  v INT NOT NULL,
  
  CONSTRAINT pk_t_composite_key_5 PRIMARY KEY (i, j, k),
  CONSTRAINT fk_t_composite_key_5_to_3 FOREIGN KEY (i) REFERENCES s_composite_key.t_composite_key_3,
  CONSTRAINT fk_t_composite_key_5_to_4 FOREIGN KEY (j, k) REFERENCES s_composite_key.t_composite_key_4
);

It produces the warning, but not the compilation error. I can reproduce the error with this test, though:

CREATE TABLE s_keys_containing_references.t_keys_containing_references_parent (
  a INT NOT NULL,
  
  CONSTRAINT pk_t_keys_containing_references_parent PRIMARY KEY (a)
); 

CREATE TABLE s_keys_containing_references.t_keys_containing_references_child (
  a INT NOT NULL,
  b INT NOT NULL,
  
  CONSTRAINT pk_t_keys_containing_references_child PRIMARY KEY (a, b),
  CONSTRAINT fk_t_keys_containing_references_child FOREIGN KEY (a) REFERENCES s_keys_containing_references.t_keys_containing_references_parent
); 

@lukaseder
Copy link
Member

Removing fk_t_composite_key_5_to_4 also produces a compilation error. Removing fk_t_composite_key_5_to_3 doesn't. Weird...

@lukaseder
Copy link
Member

The problem is that JavaGenerator::replacedByEmbeddable looks for all candidate embeddables for constraint column list that replace individual columns, but it doesn't check if the candidate embeddable really matches the input column list.

@lukaseder
Copy link
Member

3.19 Other improvements automation moved this from To do to Done Nov 16, 2023
lukaseder added a commit that referenced this issue Nov 16, 2023
lukaseder added a commit that referenced this issue Nov 16, 2023
lukaseder added a commit that referenced this issue Nov 16, 2023
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