ts-sql-query icon indicating copy to clipboard operation
ts-sql-query copied to clipboard

Problem with overloads when table is a union type

Open threema-danilo opened this issue 1 year ago • 2 comments

With ts-sql-query 1.64.0, given these table definitions with compatible columns a and b:

const tFoo = new (class TFoo extends Table<DBConnection, 'TFoo'> {
    public uid = this.autogeneratedPrimaryKey('uid', 'bigint');
    public a = this.optionalColumn('a', 'string');
    public b = this.optionalColumn('b', 'string');
    public constructor() {
        super('foo'); // Table name in the database
    }
})();

const tBar = new (class TBar extends Table<DBConnection, 'TBar'> {
    public uid = this.autogeneratedPrimaryKey('uid', 'bigint');
    public a = this.optionalColumn('a', 'string');
    public b = this.optionalColumn('b', 'string');
    public constructor() {
        super('bar'); // Table name in the database
    }
})();

...I can do the following queries:

// This works
db.selectFrom(tFoo).select({a: tFoo.a, b: tFoo.b}).where(tFoo.a.isNotNull().or(tFoo.b.isNotNull()));
db.selectFrom(tBar).select({a: tBar.a, b: tBar.b}).where(tBar.a.isNotNull().or(tBar.b.isNotNull()));

However, if I try to make this generic:

let table: typeof tFoo | typeof tBar;
if (Math.random() > 0.5) {
    table = tFoo;
} else {
    table = tBar;
}
db.selectFrom(table)
    .select({a: table.a, b: table.b})
    .where(table.a.isNotNull().or(table.b.isNotNull()));

...this is rejected by the type checker:

Argument of type 'BooleanValueSource<TABLE<DB<"DBConnection">, "TFoo">, "required"> | BooleanValueSource<TABLE<DB<"DBConnection">, "TBar">, "required">' is not assignable to parameter of type 'boolean'.
  Type 'BooleanValueSource<TABLE<DB<"DBConnection">, "TFoo">, "required">' is not assignable to type 'boolean'.ts(2345)

Should this be supported? It works without the nested .or, but also causes some other problems where the wrong overloads are picked.

threema-danilo avatar May 16 '24 11:05 threema-danilo

Hi!

Right now, what you are trying to do will not work. I'm working on taking advantage of newer TypeScript features to simplify the types, but it is still an ongoing process; in any case, even with those changes, the union of tables will not work due to the complexities of the types. I'm exploring different approaches to address this.

In one of our projects, we had a similar situation where we wanted to have several tables with a common logic in the backend. We moved forward by creating a third table mapping and forcing the cast of the previous two to that third one. It is not the most elegant solution, but it works.

Let me put in code:

Your initial tables definitions:

const tFoo = new (class TFoo extends Table<DBConnection, 'TFoo'> {
    public uid = this.autogeneratedPrimaryKey('uid', 'bigint');
    public a = this.optionalColumn('a', 'string');
    public b = this.optionalColumn('b', 'string');
    public constructor() {
        super('foo'); // Table name in the database
    }
})();

const tBar = new (class TBar extends Table<DBConnection, 'TBar'> {
    public uid = this.autogeneratedPrimaryKey('uid', 'bigint');
    public a = this.optionalColumn('a', 'string');
    public b = this.optionalColumn('b', 'string');
    public constructor() {
        super('bar'); // Table name in the database
    }
})();

The additional table definition that cannot be instantiated:

class TFooBar extends Table<DBConnection, 'TFooBar'> {
    public uid = this.autogeneratedPrimaryKey('uid', 'bigint');
    public a = this.optionalColumn('a', 'string');
    public b = this.optionalColumn('b', 'string');
    private constructor() {
        throw new Error('Do not instantiate TFooBar')
    }
}

Casting function:

function asTFooBar(table: typeof tFoo | typeof tBar): TFooBar {
    return table as any;
}

With this, your code will look like:

let table: TFooBar;
if (Math.random() > 0.5) {
    table = asTFooBar(tFoo);
} else {
    table = asTFooBar(tBar);
}
db.selectFrom(table)
    .select({a: table.a, b: table.b})
    .where(table.a.isNotNull().or(table.b.isNotNull()));

Be aware that in our case, the tables shared some fields, but each one contained additional fields not shared between the other tables; then, we created the common table as a view (to allow selects only), including only the common fields we were interested in.

Let me know if this works for you.

juanluispaz avatar May 16 '24 20:05 juanluispaz

@juanluispaz thanks, that's an interesting approach, and I think it should work for us as well!

threema-danilo avatar May 17 '24 09:05 threema-danilo