Optional relations

[allardmuis]

Issue type:

[X] Question [ ] Bug report [X] Feature request [X] Documentation issue

Database system/driver:

[X] Postgres

typed-knex version:

[X] 2.8.1 (or put your version here)

Knex.js version: 0.20.3

Steps to reproduce or a small repository showing the problem:

Hi Wouter,

I'm (once more) looking into the Typed Knex project. The current state looks quite nice and I like the new short and concise syntax. During my first test I ran into a problem, which I think is not solvable with the current version, but might just need a better explanation in the docs.

What I'm trying to achieve is to get optional relations working properly. Suppose the following knex migration:

    await knex.schema.createTable('groups', table => {
        table.uuid('id').primary();
        table.string('name');
    });

    await knex.schema.createTable('users', table => {
        table.uuid('id').primary();
        table.string('name');
        table.uuid('groupId')
            .nullable() // <-- Note the 'nullable()'
            .references('id')
            .inTable('groups');
    });

    const group1 = { id: uuidv4(), name: 'group 1' };
    const user1 = { id: uuidv4(), name: 'user 1', groupId: group1.id };
    const user2 = { id: uuidv4(), name: 'user 2', groupId: null };  // <-- this user does not have a group

    await knex.insert(group1).into('groups');
    await knex.insert(user1).into('users');
    await knex.insert(user2).into('users');

The simplest Type Knex implementation of this model would be:

@Entity('groups')
export class Group {
    @Column({ primary: true })
    public id: string;
    @Column()
    public name: string;
}

@Entity('users')
export class User {
    @Column({ primary: true })
    public id: string;
    @Column()
    public name: string;
    @Column({ name: 'groupId' })
    public group: Group;
}

But then, performing queries on this:

const user1 = await typedKnex.query(User)
    .where(i => i.name, 'user 1')
    .leftOuterJoinColumn(i => i.group)
    .select(i => [i.id, i.name, i.group.id, i.group.name])
    .getFirst();
console.log(user1);
/*
{ id: '3c152f1a-c0d5-4343-984c-c829909db133',
  name: 'user 1',
  group:
   { id: '0663bcc3-fddb-4e18-976e-ae90e12fc3c9', name: 'group 1' } }
Ok, that's fine!
*/

const user2 = await typedKnex.query(User)
    .where(i => i.name, 'user 2')
    .leftOuterJoinColumn(i => i.group)
    .select(i => [i.id, i.name, i.group.id, i.group.name])
    .getFirst();
console.log(user2);
/*
{ id: 'c40bb9b6-12b1-4021-a7fb-44b19495e72c',
  name: 'user 2',
  group: { id: null, name: null } }
Hmm....
*/

The result of user2 is really awkward. The fact that user2.group is an object instead of null is strange, and the typing of this object is incorrect because both fields in this object should be non-nullable.

Is there a way around this? The following does not work:

@Column({ name: 'groupId' })
public group: Group | null; // correct typing, but crashes the query builder

@Column({ name: 'groupId' })
public group?: Group; // somewhat correct typing, but crashes the query builder

Thanks!

[allardmuis]

In the code I found the option FlattenOption.flattenAndSetToNull. It can be used to fix the example given above. But it is a little hard to get (not exported in index.ts), needs to be repeated in every query, and does not actually work as I'd like.

What I would expect is that the property is set to null if the relation does not exist. But if the relation does exist, but you happen to only select columns that have value null, an object containing nulls is fine. I suppose this is a little difficult. A way to do this would be to always select the primary key, and set the property to null if the primary key is null in the resulting row.

[wwwouter]

Hi Allard,

Great to see you're looking into this again :)

You pinpointed the only thing that I'm not happy with. (The rest works great 😄)

The big problem is that it's hard to decide if a relation should be set to null.

For example

SELECT users.id as 'id', groups.name as 'group.name' FROM users OUTER JOIN groups ON users.groupId = groups.id

If groups.name is nullable, than we have no way to know if the property group is null, or the name property of group is null ( { id: string, group: {name: null|string}} or {id:string, group: null | {name:null|string}} )

We could automatically add groups.id to the SELECT, but I want to avoid adding implicit magic.

My current idea for v3 is a follows: if an INNER JOIN is used (also on nullable properties), the type of the result will be always be not nullable.

Eg

SELECT users.id as 'id', groups.name as 'group.name' FROM users INNER JOIN groups ON users.groupId = groups.id

Will result in { id: string, group: {name: null|string}}

And for OUTER JOIN's, an extra parameter is added to the outer join functions, where you can specify which column determines the nullability

So typedKnex.query(User).leftOuterJoinColumn(i => i.group).select(i=>[i.id, i.group.name]); will result in in { id: string, group: {name: null | string}} and typedKnex.query(User).leftOuterJoinColumn(i => i.group, g=>g.name).select(i=>[i.id, i.group.name]); will result in in { id: string, group: null |{name: null | string}}

I think a runtime check can help, so typedKnex.query(User).leftOuterJoinColumn(i => i.group, g=>g.id).select(i=>[i.id, i.group.name]); will throw an error, because group.id is not selected.

What do you think of this approach?

BTW

public group: Group | null; // correct typing, but crashes the query builder

Runtime this type is passed to the decorator as Object instead of Group, so I had to choose Group?

[allardmuis]

Hi Wouter,

Thanks for your answer. I've been thinking a bit more about this over the past few days and my conclusion is mostly in line with what you commented.

• The typing of the entity-class is not that relevant. What is relevant is the type of the result of the query. That type is dependant on the things you do in the query, so I think anything that works in the entity is fine. An option would be to use a different decorator, or give the @Column a parameter if that is useful for determining the return type.

• For inner joins there is actually no problem at all. The current version will always give the correct result and type: in the result the relation is always non-nullable, and if you only select a nullable column a result like group: { name: null } is correct.

• For left outer joins your suggestion sounds fine, but I'd also be happy with a simpler solution. Why would anyone do a left join on a relation that is not nullable? So you could say a left outer join always results in a nullable value. That would not be 100% accurate but good enough in my opinion, and I expect much simpler to build. It would even make it simpler during use: instead of always having to think about the column to pass in the seconds parameter, the field of the left join would just be nullable.

I have been trying to build a prototype for this, but typed knex is not an easy project to get started on :-)

[wwwouter]

I'd love to make a simple default that just works. But I'm not sure you're solution would work all of the time.

For example if users.groupId and groups.name are both nullable, then typedKnex.query(User).leftOuterJoinColumn(i => i.group).select(i=>[i.id, i.group.name]) / SELECT users.id as 'id', groups.name as 'group.name' FROM users OUTER JOIN groups ON users.groupId = groups.id

Can give this result:

id	group.name
1	NULL

Does that mean {id:1, group: null} or {id:1: group: { name: null }} ?

[allardmuis]

That would be great!

I would say:

• inner join gives a non-null type: { group: Group }
• inner join does not reduce to null: { group: { name: null }}
• left join gives a nullable-type: { group: Group | null }
• left join reduces to null if all values are null: { group: null }

This makes inner join work 100% correct and also the typing for the outer join. The result for outer joins will not always be correct, as you described, if you only select nullable columns. But I think it would be a great improvement of the current situation where outer joins are almost always wrong, both in typing and in result. The gap that will be left is small enough to accept in my opinion. And you can always iterate on it, for example by making the criteria for reducing to null configurable in a second parameter.

[wwwouter]

Just to keep you posted: I started work on this in https://github.com/wwwouter/typed-knex/pull/11

leftOuterJoinTableOnFunction works and I also added some comments to help my future self 😃

leftOuterJoinColumn is a bigger issue, because select only uses type information from the initial model. Foreign key properties that are optional, are handled as non-optional.

I see two solutions.

The first one is to try to change the model. If an inner join is made, change the property to NotNullable<Model<P>>, and if an outer join is made, change the property to Model<P> | null

The other solution would be to have a model and a 'select model' The model is the class that you give to the query, including all foreign key objects. The select model only has the non foreign key objects. Only after when something is joined, is it available in the select model.

The first one feels a bit like a hack. An added benefit to the second one is that it prevents you from adding columns from tables to the select clause which aren't joined yet. It does mean that you have to write joins before select, which is not how normal SQL is written.

[allardmuis]

Great!

I don't think changing the model is a hack: doing tings like joins or selects does change the type of the result. In a system like TypedKnex the Entity class is basically only a description or configuration for TypedKnex, not a real class like in a classic ORM.

But the second options sounds fine as well. It does indeed have some downsides. The order that you have to do selects and joins is different from SQL but makes sense in a programming language like JS. In code I always do the selects last. Anther downside is that you cannot do things like:

const q = typedKnex.query(...);
q.leftOuterJoinColumn(...);
q.select(...);
const r = q.getFirst();

This can be fixed most of the time by either chaining or reassigning to the variable of course. When dynamically building the query