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

Issue with Identifier Types and Composite Foreign Keys (kanel + kysely) #571

Open
dcousineau opened this issue Jun 11, 2024 · 3 comments
Open

Comments

@dcousineau
Copy link

dcousineau commented Jun 11, 2024

TLDR Found possibly an edge case, possibly an issue on my end, involving composite foreign keys and would love guidance on next steps (fix my kanel configs? learn where in kanel this could be happening an help contribute a fix? wait for a fix?)

I have recently come across an issue where Kanel (+ Kanel-Kysely) generates problematic outputs as my schema has evolved to have more complex compound foreign keys. It's possible my configurations are bad or I'm doing something very silly.

Important

I have created a code sandbox MVP reproduction to illustrate what happens. This sandbox has my exact .kanelrc.js along with the schema below.

The TLDR of the issue is when I have a moderately complex n:m type join table in situations where both sides of the join have composite primary keys. Kanel attempts to do an OR union of the "identifier" types (but for only one side of the equation) for identifier types it... doesn't actually create.

Given the following postgres schema:

CREATE TABLE organization (
  id bigserial PRIMARY KEY
);
CREATE TABLE a (
  id bigint NOT NULL,
  organization_id bigint NOT NULL REFERENCES organization(id),
  CONSTRAINT a_pkey PRIMARY KEY (id, organization_id)
);
CREATE TABLE b (
  id bigint NOT NULL,
  organization_id bigint NOT NULL REFERENCES organization(id),
  CONSTRAINT b_pkey PRIMARY KEY (id, organization_id)
);
CREATE TABLE a_b_match (
  organization_id bigint NOT NULL,
  a_id bigint NOT NULL,
  b_id bigint NOT NULL,
  CONSTRAINT a_b_match_a_fkey FOREIGN KEY (organization_id, a_id) REFERENCES a(organization_id, id),
  CONSTRAINT a_b_match_b_fkey FOREIGN KEY (organization_id, b_id) REFERENCES b(organization_id, id),
  CONSTRAINT a_b_match_pkey PRIMARY KEY (organization_id, a_id, b_id)
);

And a .kanelrc.js almost entirely based on https://github.com/kristiandupont/kanel/blob/main/example/.kanelrc.js, we get the following outputs.

Note

Specifically notice how ABMatch.ts is attempting to import BOrganizationId from B.ts and union it with OrganizationId, however B.ts doesn't actually define or export BOrganizationId

In an ideal world, Kanel would not attempt to create BOrganizationId nor would it attempt to union it with OrganizationId.

ABMatch.ts

// @generated
// This file is automatically generated by Kanel. Do not modify manually.

import { type OrganizationId } from './Organization';
import { type BOrganizationId, type BId } from './B';
import { type AId } from './A';
import { type ColumnType, type Selectable, type Insertable, type Updateable } from 'kysely';

/** Represents the table public.a_b_match */
export default interface ABMatchTable {
  organization_id: ColumnType<OrganizationId | BOrganizationId, OrganizationId | BOrganizationId, OrganizationId | BOrganizationId>;

  a_id: ColumnType<AId, AId, AId>;

  b_id: ColumnType<BId, BId, BId>;
}

export type ABMatch = Selectable<ABMatchTable>;

export type NewABMatch = Insertable<ABMatchTable>;

export type ABMatchUpdate = Updateable<ABMatchTable>;

B.ts

// @generated
// This file is automatically generated by Kanel. Do not modify manually.

import { type OrganizationId } from './Organization';
import { type ColumnType, type Selectable, type Insertable, type Updateable } from 'kysely';

/** Identifier type for public.b */
export type BId = string;

/** Represents the table public.b */
export default interface BTable {
  id: ColumnType<BId, BId, BId>;

  organization_id: ColumnType<OrganizationId, OrganizationId, OrganizationId>;
}

export type B = Selectable<BTable>;

export type NewB = Insertable<BTable>;

export type BUpdate = Updateable<BTable>;
@kristiandupont
Copy link
Owner

Hmm, yes, I see the problem but I admit that I am not sure where in the chain the bug lies.
As with many other issues, I won't promise when I can get around to debugging it but I would like to fix this as it does appear to be a bug and not some out-of-scope feature request. If this is blocking you, a post-render hook is probably the easiest way to get around it

@dcousineau
Copy link
Author

Sounds great. Except for me managing to stub my toes here kanel has been excellent to me.

I'll try the post render hook first just to bandaid the situation. If you have any examples handy (that aren't already in this repo) that would be helpful, but if not no worries!

I'll also keep you posted if I am able to debug or chase the issue down myself.

@dcousineau
Copy link
Author

dcousineau commented Jun 28, 2024

Something I just noticed because I bumped to 3.9.0 2 weeks ago (but hadn't had cause to re-run type generation) is I am now getting a lot of "Could not resolve circular reference" warnings (from this line:

console.warn("Could not resolve circular reference", reference);
) which was from #540 and the following error:

TypeError: Cannot read properties of undefined (reading 'join')
    at writeFile (./node_modules/kanel/build/writeFile.js:41:27)
    at ./node_modules/kanel/build/processDatabase.js:78:60
    at Array.forEach (<anonymous>)
    at processDatabase (./node_modules/kanel/build/processDatabase.js:78:18)
    at async main (./node_modules/kanel/build/cli/main.js:120:9

(Upon further inspection the writeFile method has a lines argument set to undefined rather than an array)

So this is a clue at least, I can't spend much more time today digging in but I'll report back when I get the chance to spend more time on it.

It appears the issue stems from a parent FKEY referencing columns in a child who are themselves also an FKEY. I'm still unsure if the composite FKEY aspect comes into play.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants