graphile / pg-aggregates

Aggregates for PostGraphile connections
78 stars 17 forks source link

Aggregate keys should be nullable #53

Closed benjie closed 11 months ago

benjie commented 11 months ago

Reported via chat:

A table with a nullable column, using the pg-aggregates plugin, has the potential to error when retrieving the keys resource.

For the pg-aggregates plugin, when one does a a groupedAggregates query, the keys may not be null (https://github.com/graphile/pg-aggregates/blob/6265db96cff64f483c8202ccdbecf131cc7d2f9b/src/AddAggregateTypesPlugin.ts#L55-L56).

However, the query is able to select nullable fields to group by. This can result in unexpected errors. Is there a reason why keys needs to be non-null? If possible, could keys be made nullable?

Example table, query, data, and output is here: https://gist.github.com/ProbablyBrianBurgess/10f5138192936994ba8c6574e77452d0

The last item results with:

{
    "keys": null,
    "distinctCount": {
        "a": "1"
    }
}

This is better than a crash, but could keys be [null, "2", "6"] (based on the example data)? Although certainly not an exhaustive test, I did remove the new GraphQLNonNull portion and it worked as desired.

benjie commented 11 months ago

Fixed in bd374bb