Is there a way to avoid confusing with different foreign keys?

[Newbie012]

I have two simple tables:

CREATE TABLE agencies (
    id SERIAL PRIMARY KEY
    ....
);

CREATE TABLE users (
    id SERIAL PRIMARY KEY,
    agency_id INT NOT NULL REFERENCES agencies (id)
    first_name TEXT NOT NULL,
    last_name TEXT NOT NULL
);

A single agency has many users. This is pretty straightforward. So if I type:

export const getAgencyUsers = sql`
   SELECT * FROM users WHERE agency_id = $agencyId
`;

Then $agencyId would be evaluated as number for typescript, which is perfectly fine. But when the application grows, we can mistakenly pass the wrong entity id (e.g user id instead of agency id) and since typescript evaluates both user id and agency id as numbers, so we can easily get mistaken. At my work, we don't use pgtyped, we've got something else. But the thing is when it comes to column keys, we treat each key as a different class. So user id would be UserId and agency id would be AgencyId. This way, we can never make a mistake. For example

export class UserId {
    private constructor() { }

    "UserId": UserId;

    public static wrap(id: number): UserId {
        return <any>id;
    }

    public static unwrap(userId: UserId): number {
        return <any>userId;
    }
}

so if by mistake I passed the wrong key (let's say $agencyId instead of $userId, then I would get a type check error like so:

I was wondering if this is possible to achieve with pgtyped as well.

[golergka]

I thought that something like this wouldn't be achievable in Typescript because of duck typing and implicit conversions. How exactly does this work? In the code sample above, wrap and unwrap methods don't seem to be valid Typescript.

[Newbie012]

@golergka To be honest I'm not quite sure. The library + extension are open sourced (You can see here - https://github.com/MedFlyt/mfsqlchecker) As for the wrap and unwrap, if you're talking about the <any>userId, it's equivalent to userId as any.

You basically defined your unique types in a mfsqlchecker.json file (see in the demo folder) and then the library maps the unique type with the tableName and the columnName (You can find it in DbConnector.ts async validateManifest)

[sfsr12]

I didn't look into the details of this - but I imagine you would need to use something like this: https://github.com/sindresorhus/type-fest/blob/master/source/opaque.d.ts

Since there is no built in support for opaque types in Typescript as of right now.

[Newbie012]

@sfsr12 Thank you, but I'm already aware of this library and opaque types in general. The problem I presented doesn't have to do with how to implement opaque types, but more like how can I "refer" a foreign key as an opaque type and make sure pgtyped will be able to understand that.

[golergka]

Although a static type lover in me adores this level of correctness checking, I don't think that it's implementable without breaking backwards compatibility and making most of the library users upset - handling IDs as plain numbers is a common (although a slightly unhealthy) habit.

Instead, the approach that I've seen was to use a coding convention that takes advantage of Javascript's syntax sugar for object properties:

const getUser = sql<IGetUserQuery>`SELECT * FROM users WHERE id = $userID`

async function printUserName(db: PoolClient, userID: number) {
    const user = await getUser.run({
            userID // <- notice this
        }, db)
    console.log(`user's name is ${user.name}`)
}

In the project I'm currently working on, we user names like userID in almost all places where we handle row ids, and the fact that the function parameter and query parameter have the same exact name in the code above allows using this sugar — to the extent that if we don't use it, it instantly brings attention to itself.

Of course, this is no compiler guarantee, but we haven't had a single issue with confusing id columns of different tables with one another.

[Newbie012]

@golergka Thanks for the tip. And still, I'll try to draw a scenario where this can get out of hand. Take this code for example:

const updateUserCashQuery= sql<IUpdateUserCashQuery>`UPDATE users SET cash = $cash WHERE id = $userId`

async function updateUserCash(db: PoolClient, userId: number, cash: number) {
    await updateUserCashQuery.run({userId, cash}, db)
}

Looks fine, no chance that updateUserCash will pass the wrong params to the query. But what about the ones that call updateUserCash? This can quickly get out of hand when working with multiple devs on functions that have lots of params. Of course, we can convert the params to a single one that should be an object, but that will require us to write an undesirable boilerplate for each function (a separate or an inline interface).

About backward compatibility, can't we make a flag that it turned off by default (I hope?).

---

Another library that adds typescript types from pg schema that I found today is kristiandupont/kanel Which translate keys as "opaque" types:

// Automatically generated. Don't change this file manually.
// Name: actor

export type ActorId = number & { __flavor?: 'actor' };

export default interface Actor {
  /** Primary key. Index: actor_pkey */
  actor_id: ActorId;
  ...
}

taken from https://github.com/kristiandupont/kanel/blob/master/example/models/Actor.ts

[golergka]

You know, as it often happens when you get very self-confident, I just had to debug a nasty issue that originated exactly from this kind of confusion. In the code that I wrote personally, all the way. So, even if you didn't post this (pretty convincing, by the way) comment, I just had to get back to this discussion and humbly concede that we probably do, indeed, need a more rigid way to prevent this kind of bugs.

[Newbie012]

Although I'm unfamiliar with how pgtyped works behind the scenes, let me know if there's a way I can contribute to this feature.

[adelsz]

Good discussion here! Proper opaque types is something I really miss from the Flow typechecker. With pgTyped it will only be possible to add such conversions to the .sql file based approach we support. One way to do it is to introduce a new @override tag, something like this:

/*
  @name selectSomeUsers
  @override(result.id -> UserId)
  @override(param.id -> UserId)
*/
SELECT id, first_name, age FROM users WHERE userId = :userId;

In the above UserId is a custom opaque type, which can be placed in a "type repository" file referenced by the pgTyped config.

[Newbie012]

@adelsz that's a nice solution that will definitely solve a lot of problems. Although, I believe it will be harder to maintain hundreds of queries like these mainly because:

1. When we manually declare something, we can still make a mistake by passing the wrong opaque.
2. When we change (in rare cases) an opaque variable name, the IDE won't be able to refactor the commented overrides.

I'm not sure if it's possible, but if we could automate the conversion, it would greatly increase type safety and development velocity. I haven't digged in much in how pgtyped work, but as far as I understood from my debugging experience, I saw that we convert pg types to ts types by a map.

I don't know if it's possible to fetch column references from a given schema without getting your hands dirty, but if it's too complicated, then we can try this approach:

Assuming the column with the name "id" is the primary key, we can say that the TS type (opaque) name should be the named as the table name in singular with an Id suffix (e.g users -> user -> User -> UserId`)

When it comes to columns that that their name ends with _id, then we can do the same approach.

I believe there are better non-fragile approaches than my suggestion, but I thought pointing that out might be useful.

[smallnamespace]

You should consider ts-brand, seems tailor-made for this use case.

It can be a compile-time flag whether to use branded types or not, to preserve backwards compatibility.

[Newbie012]

@smallnamespace the problem is that pg-typed is not aware of the brand types, therefore it cannot autogenerate the typing with the brand types.

[smallnamespace]

@Newbie012 Sorry I wasn't clear, I'm suggesting that pg-typed generate branded types.

[adelsz]

Closing as stale. Branded types kind of go against the design goals of Typescript and there doesn't seem to be much demand for them.