graphql-engine icon indicating copy to clipboard operation
graphql-engine copied to clipboard

Allow String query variable to be used for citext fields too

Open lionelB opened this issue 2 years ago • 3 comments

Version Information

Server Version: v2.12.0-ce

Environment

OSS

What is the current behaviour?

I have a query for search. the query search in multiple fields to match a maximum of record. Some field are in varchar and some other are citext. I can use the same variable

What is the expected behaviour?

Since citext is text, It should able to use the same variable to search in both text / citext fields

How to reproduce the issue?

  1. create tables
CREATE TABLE structure(  
  id uuid PRIMARY KEY,
  name citext NOT NULL, 
);
CREATE TABLE user( 
  user_id uuid PRIMARY KEY,
  firstname varchar(25) NOT NULL,
  lastname varchar(30) NOT NULL
  structure_id uuid FOREIGN KEY ("structure_id") REFERENCES user("id") ON UPDATE restrict ON DELETE cascade,
);
  1. open Api explorer
query search($search: String) {
  user(
    where: {
      _or: [
        { firstname: { _ilike: $search } }
        { lastname: { _ilike: $search } }
        { structure: { name: { _ilike: $search } } } # structure.name use citext type
      ]
    }
  ) {firstname, lastname}
}

Screenshots or Screencast

image

Please provide any traces or logs that could help here.

the query produces the following error

{
  "errors": [
    {
      "extensions": {
        "code": "validation-failed",
        "path": "$.selectionSet.user.args.where.structure.name._eq"
      },
      "message": "variable 'search' is declared as 'String', but used where 'citext' is expected"
    }
  ]
}

Any possible solutions?

Can you identify the location in the source code where the problem exists?

no, (I don't know haskell)

If the bug is confirmed, would you be willing to submit a PR?

no (I don't know haskell)

Keywords

citext

lionelB avatar Oct 06 '22 08:10 lionelB

Hello @lionelB

Thanks for reporting. I was able to reproduce it. Although, I fixed that error with the following workaround.

Temporary workaround

We can have another variable for citext search and then pass type citext to it instead of String. Then it works without throwing any errors.

query search($search: String,$citext_search:citext) {
    user_test(
    where: {
      _or: [
        { firstname: { _ilike: $search } }
        { lastname: { _ilike: $search } }
        { structure_test: { name: { _ilike: $citext_search } } } # structure.name use citext type
      ]
    }
  ) {firstname, lastname}
}

Query variables

{"search": " ","citext_search":" "}

I think there are some bits missing here for steps of reproduction. Although I am adding them here how I was able to repro.

Steps to reproduce

1. Add tables and relationship:

First create structure table

CREATE TABLE structure(id uuid PRIMARY KEY, name citext NOT NULL);

Then create user table

CREATE TABLE public."user"(user_id uuid PRIMARY KEY,firstname varchar(25) NOT NULL,lastname varchar(30) NOT NULL,structure_id uuid NOT NULL );

Add structure_id as foreign key to that of structure.id

ALTER table public.user
ADD CONSTRAINT structure_id FOREIGN KEY ("structure_id") REFERENCES structure("id") ON UPDATE restrict ON DELETE cascade

Create relationship from user.structure_id to [structure.id](http://structure.id/) and name it as structure.

Perform search query

query search($search: String) {
      user(
	      where: {
		      _or: [
			      { firstname: { _ilike: $search } }
			      { lastname: { _ilike: $search } }
			      { structure: { name: { _ilike: $search } } } # structure.name use citext type
		      ]
	      }
      ) {
	      firstname
	      lastname
      }
}

This will throw error


{
  "errors": [
    {
      "extensions": {
        "code": "validation-failed",
        "path": "$.selectionSet.user.args.where._or[2].structure.name._ilike"
      },
      "message": "variable 'search' is declared as 'String', but used where 'citext' is expected"
    }
  ]
}

meetzaveri avatar Oct 06 '22 10:10 meetzaveri

If someone from the native-database team would be able to verify if this is intended or not, then it would be very helpful. Thanks !

meetzaveri avatar Oct 06 '22 11:10 meetzaveri

@meetzaveri thanks for the reply! I've done exactly the same workaround :)

lionelB avatar Oct 06 '22 13:10 lionelB