Graphback: Possible bug in knex query mapping

[ssd71]

On giving a filter to the knexQueryMapper like so:

{
  "filter": {
    "title": {
      "eq": "one"
    },
    "and": [
      {
        "description": {
          "contains": "two"
        }
      }
    ],
    "or": [
      {
        "title": {
          "contains": "three"
        }
      }
    ],
    "not": {
      "description": { "eq": "four"}
    }
  }
}

Produces a query like so:

select * where "title" = "one" and "description" like "two" or "title" like "three" and not "description" = "four";

which, if I remember correctly is evaluated like so:

select * where ("title" = "one" and "description" like "two") or ("title" like "three" and (not "description" = "four"));

I was expecting something like:

select * where (("title" = "one" and "description" like "two") and (not "description" = "four")) or ("title" like "three");

Is this intentional or perhaps I missed something? :)

[craicoverflow]

The query is build in the order of the keys in the filter object, so the output query will match that.

Two ways to resolve this..

Order your operators the same way you expect the SQL to format to:

{
  "filter": {
    "title": {
      "eq": "one"
    },
    "and": [
      {
        "description": {
          "contains": "two"
        }
      }
    ],
    "not": {
      "description": { "eq": "four" }
    },
    "or": [
      {
        "title": {
          "contains": "three"
        }
      }
    ]
  }
}

A better way to do this would be to nest your NOT operator as a child of OR:

{
  "filter": {
    "title": {
      "eq": "one"
    },
    "and": [
      {
        "description": {
          "contains": "two"
        }
      }
    ],
    "or": [
      {
        "title": {
          "contains": "three"
        }
      },
      "not": {
        "description": { "eq": "four" }
      },
    ]
  }
}

[wtrocki]

@craicoverflow I guess that format we get from GraphQL is just tradeof to minimize amount of objects we will have. Tradeof is that we cannot have nested graph for this. We can have number of and and or with no ability to put () into them. It put some mental overhead on person to build next operator basing on boolean result of the previous operator. Cannot we nest and or inside another and?

[craicoverflow]

We can have number of and and or with no ability to put () into them.

Can you explain what you mean with an example? What is ()?

Yes we can nested and/or inside another and.

[wtrocki]

Nestability.. So when we do if in code we can do v1 && (v2 || v3) which is different than v1 && v2 || v3.

[ssd71]

Order your operators the same way you expect the SQL to format to:

@craicoverflow Thanks!, I was also a bit confused if we should rely on the order of keys in a map to construct our query, this way, as @wtrocki says it also requires some thought to be put into writing queries

EDIT: Bad call on my part, it seems like the order of keys stays same most of the time

[wtrocki]

So instead of building queries like this:

{
  "filter": {
    "title": {
      "eq": "one"
    },
    "and": {
        "description": {
          "contains": "two"
        }
    }
  }
}

we can do this?

{
  "filter": {
    "and": {
        "description": {
          "contains": "two"
        },
        "title": {
          "eq": "one"
        }
    }
  }
}

[craicoverflow]

Yes, that is the safest way of ensuring the order of the SQL query.

[wtrocki]

ah.. so there is no way to prevent people from doing it other way as graphql syntax allows it, however we should generate client side queries this way to enforce compliance and great standard.

CC @Eunovo

[machi1990]

Yes, that is the safest way of ensuring the order of the SQL query.

Is this documented somewhere? Maybe it is something we can recommend.

[craicoverflow]

Yes, that is the safest way of ensuring the order of the SQL query.

Is this documented somewhere? Maybe it is something we can recommend.

It is not documented anywhere yet as this stuff is still in an early phase, but we should document this as being the best way to do it.

[machi1990]

Yes, that is the safest way of ensuring the order of the SQL query.

Is this documented somewhere? Maybe it is something we can recommend.

It is not documented anywhere yet as this stuff is still in an early phase, but we should document this as being the best way to do it.

Amazing!

[wtrocki]

but we should document this as being the b̶e̶s̶t̶ only way to do it.