graphile / pg-aggregates

Aggregates for PostGraphile connections
83 stars 17 forks source link

GroupBy computed columns not generated ? #28

Closed orieuxe closed 1 year ago

orieuxe commented 2 years ago
create or replace function doe_indice(doe public.doe) returns text
    stable
    strict
    language sql
as
$$
    select ...;
$$;
comment on function doe_indice(doe public.doe) is E'indice de validation';

In schema.graphql :

type Doe implements Node {
  #...
"""indice de validation"""
  indice: String
  #...
}

enum DoeGroupBy {
  #...
  # INDICE not found :(
  #...
}

It says it should be available in the README :

We also support grouping your data via the value of one of your columns, no-additional-arguments computed columns

If i try to return an integer i see only it generated in the DoeHavingXXXInput inputs but not in the GroupBy. Am i missing something ?

dpmccabe commented 2 years ago

I can confirm that computed columns are not available in groupedAggregates. Unfortunately I don't know graphql nearly well enough to figure out how to add this feature myself. A possible workaround would be to use a real column (maybe with values populated by a database trigger) instead of a computed column.

orieuxe commented 2 years ago

Hi,

Thanks, I ended up doing it with plain postgresql since it allows to group_by functions. I didn't want to add a column to that table since it's already quite a big one x)

benjie commented 2 years ago

We'd need a plugin like this one: https://github.com/graphile/pg-aggregates/blob/966914534deb98bfda4f0ce8f071c984888ac221/src/AddGroupByAggregateEnumValuesForColumnsPlugin.ts but that looks at procs rather than columns. There's a bit of effort involved to do that; I honestly thought we had already done it but you're right we haven't. I'll remove it from the readme.

benjie commented 1 year ago

[semi-automated message] Hi, there has been no activity in this issue for a while so I'm closing it to keep the issues/pull requests manageable. If you're interested in taking this feature on, let me know.