AnyhowStep / tsql

A SQL query-builder/ORM
37 stars 3 forks source link

Fix GROUP BY "cover" problem #171

Open AnyhowStep opened 4 years ago

AnyhowStep commented 4 years ago

const myTable = tsql.table("myTable")
    .addColumns({
        myTableId : tm.mysql.bigIntSigned(),
        myColumn : tm.mysql.bigIntSigned(),
        myColumn2 : tm.mysql.bigIntSigned(),
        stringColumn : tm.mysql.varChar(),
    });

tsql
    .from(myTable)
    .select(columns => [
        columns.myColumn,
    ])
    .orderBy(columns => [
        columns.myTableId.asc(),
    ])
    /**
     * Whoops! We now have an invalid query!
     */
    .groupBy(columns => [
        columns.myColumn2,
    ]);

The call to .groupBy() is invalid. It should contain myColumn and myTableId. Otherwise, the entire query is invalid.

However, we have no such constraint at the moment.


If the .groupBy() is done before the .select(), we're actually okay

AnyhowStep commented 4 years ago

I would really want to be able to defer the call to .groupBy(), and then check that the GROUP BY columns cover the columns used by non-aggregate expressions in the ORDER BY and SELECT clauses, if any.

However, we will first need to expose ORDER BY type information. Right now, the information is erased...

AnyhowStep commented 4 years ago

First thing we need to do is change the typedef for OrderByClause. Right now, it is,

export type OrderByClause = readonly Order[];

We need something like,

export interface OrderByClause<NonAggregateUsedRefT extends UsedRef> {
  readonly orders : readonly Order[]; 
  readonly nonAggregateUsedRef : NonAggregateUsedRefT,
}

An Order can have an aggregate or non-aggregate SortExpr. If it is non-aggregate, we need to keep track of the UsedRef for the GROUP BY cover problem.