crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Stored procedure is neither sortable nor filterable, although commented as @sortable and @filterable

Open TimoStolz opened this issue 4 years ago • 3 comments

Summary

In GraphiQL, the fields orderBy and condition are missing for the stored procedure search_powerups(varchar) (see below), although it is properly commented as @sortable and @filterable.

Steps to reproduce

#!/usr/bin/env bash
# stolen from https://github.com/graphile/postgraphile/issues/1469#issuecomment-840468985
set -e
DATABASE_NAME="db_9ec0daec_e399_11eb_9008_47882ecae7df"
SCHEMA_NAME="public"

yarn add --dev postgraphile@next
dropdb --if-exists "$DATABASE_NAME"
createdb "$DATABASE_NAME"
psql -X1v ON_ERROR_STOP=1 "$DATABASE_NAME" <<HERE

create extension pg_trgm;

create table powerups
(
    id         uuid primary key default gen_random_uuid(),
    name       varchar not null,
    created_at timestamp default current_timestamp
);

create index powerups_on_fuzzy_name on powerups using gist (name gist_trgm_ops);

insert into powerups (name) values ('cooking'), ('skating'), ('swimming'), ('coding');

---

create type powerup_search_result as (
    similarity real,
    word_similarity real,
    powerup powerups
);

create or replace function search_powerups(by_name varchar)
returns setof powerup_search_result
as \$\$
    select
        similarity(by_name, p.name) as similarity,
        word_similarity(by_name, p.name) as word_similarity,
        p as powerup
    from powerups as p
\$\$ language sql stable strict;

comment on function search_powerups(varchar) is E'@filterable\n@sortable';

HERE

# Recommended options from https://www.graphile.org/postgraphile/usage-cli/
yarn postgraphile \
  --subscriptions \
  --watch \
  --dynamic-json \
  --no-setof-functions-contain-nulls \
  --no-ignore-rbac \
  --show-error-stack=json \
  --extended-errors hint,detail,errcode \
  --export-schema-graphql schema.graphql \
  --graphiql "/" \
  --enhance-graphiql \
  --allow-explain \
  --enable-query-batching \
  --legacy-relations omit \
  --connection "$DATABASE_NAME" \
  --schema "$SCHEMA_NAME"

Expected results

The endpoint searchPowerups should have both an orderBy and a condition field.

Actual results

Both orderBy and condition are missing.

Additional context

All provided above

Possible Solution

Unfortunately, no idea

TimoStolz avatar Jul 13 '21 05:07 TimoStolz

There is a workaround. On discord, @benjie said

it doesn’t have to be a named type, it has to be a selectable (table, view, etc) type. We only generate order/condition for “selectable” types currently: https://github.com/graphile/graphile-engine/blob/v4/packages/graphile-build-pg/src/plugins/PgConnectionArgCondition.js#L21

So I adapted my source and it finally worked.

#!/usr/bin/env bash
# stolen from https://github.com/graphile/postgraphile/issues/1469#issuecomment-840468985
set -e
DATABASE_NAME="db_9ec0daec_e399_11eb_9008_47882ecae7df"
SCHEMA_NAME="public"

yarn add --dev postgraphile@next
dropdb --if-exists "$DATABASE_NAME"
createdb "$DATABASE_NAME"
psql -X1v ON_ERROR_STOP=1 "$DATABASE_NAME" <<HERE

create extension pg_trgm;

create table powerups
(
    id         uuid primary key default gen_random_uuid(),
    name       varchar not null,
    created_at timestamp default current_timestamp
);

create table powerup_search_results (
    similarity real,
    word_similarity real,
    powerup powerups
);

-- instead of
-- create type powerup_search_result as (
--    similarity real,
--    word_similarity real,
--    powerup powerups
-- );

create index powerups_on_fuzzy_name on powerups using gist (name gist_trgm_ops);

insert into powerups (name) values ('cooking'), ('skating'), ('swimming'), ('coding');

---

create or replace function search_powerups(by_name varchar)
returns setof powerup_search_results
as \$\$
    select
        similarity(by_name, p.name) as similarity,
        word_similarity(by_name, p.name) as word_similarity,
        p as powerup
    from powerups as p
\$\$ language sql stable strict;

comment on function search_powerups(varchar) is E'@filterable\n@sortable';

HERE

# Recommended options from https://www.graphile.org/postgraphile/usage-cli/
yarn postgraphile \
  --subscriptions \
  --watch \
  --dynamic-json \
  --no-setof-functions-contain-nulls \
  --no-ignore-rbac \
  --show-error-stack=json \
  --extended-errors hint,detail,errcode \
  --export-schema-graphql schema.graphql \
  --graphiql "/" \
  --enhance-graphiql \
  --allow-explain \
  --enable-query-batching \
  --legacy-relations omit \
  --connection "$DATABASE_NAME" \
  --schema "$SCHEMA_NAME"

TimoStolz avatar Jul 13 '21 06:07 TimoStolz

I would still prefer if it also worked with types. With fake tables, it feels somewhat hacky, and it causes additional care to prevent inserts on them. Probably, there could be a @selectable tag for types.

TimoStolz avatar Jul 13 '21 06:07 TimoStolz

I'm not sure if a selectable tag is the right approach, but there should be something. The risk of unilaterally defining the conditions/orders for all types is that we increase risk of name collisions even though they're not necessarily even reaching the final schema. We could try and "guess" if the type is going to be used (e.g. it's the return type of a function that's tagged in the relevant way) but that kinda sucks.

What should really happen is that the function should be able to request the condition/order type for the given type it uses and is planning to put into the schema, and that type is generated just in time if it doesn't already exist. This is the pattern that I've been trying to use for newer code in Graphile but alas it'll be quite a big lift to refactor the existing code to do this I think.

benjie avatar Jul 14 '21 16:07 benjie

V5 solves this via the behavior system - we generate the filter types for the codecs that have the filterBy behavior.

benjie avatar Oct 04 '23 18:10 benjie