HPC-8569: Add count function and distinct option to models library

[manelcecs]

This PR aims to add support for common database functions, such as:

• count
• distinct
• orderBy: 'nulls last' (delayed until Knex gets updated)

This changes applies to the previously defined function: 'find' - and add a new function: 'count'

[Pl217]

Thanks for adding these new capabilities to our model library. New options are awesome!

I didn't even know about some possibilities, like DISTINCT ON, which made me read a lot of documentation and play around with PostgreSQL. I did spend entire afternoon researching about various options and thinking about our needs, much more than I anticipated when I started review :/

My responses are long and detailed, but addressing my review comments should take little time.

[Pl217]

orderByNullsLast should be an option in OrderByCond type, because it is related to ordering, not directly related to find. What's more, if we look at Knex docs for orderBy function, we see that it has single and multi-column variant.

If you look at this part of our code

hpc-api-core/src/db/util/raw-model.ts

Lines 175 to 180 in 8f51415

	if (orderBy !== undefined) {
	if (!Array.isArray(orderBy)) {
	orderBy = [orderBy];
	}
	query.orderBy(orderBy);
	}

you can see that we're always using multi-column approach, because we are defining OrderByCond as an object:

hpc-api-core/src/db/util/raw-model.ts

Lines 31 to 34 in 8f51415

	export type OrderByCond<F extends FieldDefinition> = {
	column: keyof InstanceDataOf<F>;
	order?: 'asc' | 'desc';
	};

Knex docs give this example:

knex('users').orderBy([
  { column: 'email' },
  { column: 'age', order: 'desc', nulls: 'last' },
]);

So, since we already support column and order keys, we should just add third optional key nulls?: 'first' | 'last'; to OrderByCond and this feature will be fully usable with our models.

[manelcecs]

I did not found that example because that is what we needed - definitely I'll be adding that change.

[Pl217]

Crap, we cannot use nulls option, because it only exists in newer versions of Knex. The feature was added by this PR, which was released in v0.95.12 of Knex.

There was also a bug with the implementation in PostgreSQL, which was resolved in this PR. That was released in v1.0.2, so we may need to update to at least that version.

I think we should update now and HPC-8383 should be a blocker for this work, because we are long overdue to update Knex, since we are at v0.21.1, released 28 April, 2020!

[Pl217]

We cannot add nulls option until we update to a more recent version of Knex, as I wrote in this comment.

If you want you can drop the first commit which adds nulls option and merge the rest (after addressing really minor comments I have inline now), but you may also want to block the entire work on HPC-8569 until Knex is updated, because you cannot add nulls last to your queries.

Discuss with Kashif (once he's back next week) whether we want to update Knex now and block this work. Otherwise, I would implement nulls last functionality in code and write a TODO comment to refactor it once we can add that option in models library.

[manelcecs]

We cannot add nulls option until we update to a more recent version of Knex, as I wrote in this comment.

If you want you can drop the first commit which adds nulls option and merge the rest (after addressing really minor comments I have inline now), but you may also want to block the entire work on HPC-8569 until Knex is updated, because you cannot add nulls last to your queries.

Discuss with Kashif (once he's back next week) whether we want to update Knex now and block this work. Otherwise, I would implement nulls last functionality in code and write a TODO comment to refactor it once we can add that option in models library.

@Pl217 After discuss this issue with Kashif - We have decided to drop the commit and after the implementation of the new version of Knex we update both HPC-API-CORE in order to add the 'nulls last' functionality and after, update HPC-API to add the order to the needed queries.