Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query variable for query block not recognised after upgrading to 1.1.0 #4038

Closed
akamos opened this issue Sep 21, 2019 · 10 comments
Closed

Query variable for query block not recognised after upgrading to 1.1.0 #4038

akamos opened this issue Sep 21, 2019 · 10 comments
Assignees
Labels
area/querylang Issues related to the query language specification and implementation. kind/bug Something is broken. priority/P0 Critical issue that requires immediate attention. status/accepted We accept to investigate/work on it.
Milestone

Comments

@akamos
Copy link

akamos commented Sep 21, 2019

What version of Dgraph are you using?

1.1.0 and master

Have you tried reproducing the issue with the latest release?

Yes.

What is the hardware spec (RAM, OS)?

MacBook Pro 15, Mac OS Mojave

Steps to reproduce the issue (command/config used to run Dgraph).

Load the schema and data from tutorial in a clean database (http://tour.dgraph.io/intro/3/ and http://tour.dgraph.io/intro/3/) and execute following query:

{
  count as personCount(func:type(Person)) {
    total: count(uid)
  }
  
  allPerson(func:uid(count)) @filter(eq(name, "Michael")) {
    name
  }
  
}

Produces following error:

Error Name: t

Message: Some variables are used but not defined
Defined:[]
Used:[count]

In comparison following query works fine.

{
  count as personCount(func:type(Person)) {
  
  }
  
  personCount2(func:uid(count)) {
    total: count(uid)
  }
  
  pagedPersons(func:uid(count)) @filter(eq(name, "Michael")) {
    name
  }
  
}

and produces that result:

{
  "data": {
    "personCount": [],
    "personCount2": [
      {
        "total": 8
      }
    ],
    "pagedPersons": [
      {
        "name": "Michael"
      }
    ]
  },
  "extensions": {
    "server_latency": {
      "parsing_ns": 103800,
      "processing_ns": 6377400,
      "encoding_ns": 42900,
      "assign_timestamp_ns": 2070700
    },
    "txn": {
      "start_ts": 12
    }
  }
}


URL: http://localhost:8080/query?timeout=60s

Expected behaviour and actual result.

The variable should be recognised and the query successfully executed. In Version 1.0.16 this still worked.

@akamos akamos changed the title Query variable for query block not recognised Query variable for query block not recognised after upgrading to 1.1.0 Sep 21, 2019
@mangalaman93 mangalaman93 added area/querylang Issues related to the query language specification and implementation. kind/bug Something is broken. priority/P0 Critical issue that requires immediate attention. labels Sep 21, 2019
@MichelDiz
Copy link
Contributor

MichelDiz commented Sep 21, 2019

For now, there is a way around this (I just noticed that you are already using it*). If you execute the query below in https://play.dgraph.io/?latest you gonna have the expected result. This happens cuz the count() function affects the overall result of that block. So you need to explicitly add the variable inside the block to the UID pred or create separated blocks for each propose (as you did).

This approach below has no performance losses or any other issues.

{
    count as var(func:type(Actor))

    actorCount(func:uid(count))  {
    total: count(uid)
  }

  q(func:uid(count)) @filter(eq(name@en, "Michael")) {
    expand(_all_)
  }
}
{
  "data": {
    "actorCount": [
      {
        "total": 478908
      }
    ],
    "q": [
      {
        "name@en": "Michael"
      },
      {
        "name@en": "Michael"
      },
      {
        "name@en": "Michael",
        "name@ko": "마이클",
        "name@ro": "Michael"
      },
      {
        "name@en": "Michael",
        "name@hi": "माइकल"
      },
      {
        "name@en": "Michael"
      },
      {
        "name@en": "Michael"
      },
      {
        "name@en": "Michael",
        "name@fr": "Michael",
        "name@ja": "まいける"
      },
      {
        "name@en": "Michael"
      }
    ]
  },
  "extensions": {
    "server_latency": {
      "parsing_ns": 9302,
      "processing_ns": 68891161,
      "encoding_ns": 73507892
    },
    "txn": {
      "start_ts": 1366718
    }
  }
}

@akamos
Copy link
Author

akamos commented Sep 22, 2019

Thanks for the quick response. I'm waiting for the fix. Do you plan to do have it ready until 1.1.1 already?

@MichelDiz
Copy link
Contributor

No plans, it has 23 hours :P - but it's priority/P0, which is good.

Cheers.

@mangalaman93 mangalaman93 self-assigned this Sep 23, 2019
@mangalaman93
Copy link
Contributor

I still am looking into this, seems like we do not allow query alias along with alias for count() subgraph OR multiple count() aliases in one query.

@mangalaman93
Copy link
Contributor

I tried the following query -

{
  count as personCount(func:type(Person)) {
    total: count(uid)
    total_count1 as count(uid)
    total_count2 as count(uid)
  }
  
  allPerson(func:uid(total_count2)) @filter(eq(name, "Michael")) {
    name
  }
}

It should fail with parsing errors saying that we have defined more variables than in use. But it runs successfully. Trying to figure out what is the right design here, whether we support something like this.

@campoy campoy added the status/accepted We accept to investigate/work on it. label Sep 24, 2019
@campoy campoy added this to the Dgraph v1.1.1 milestone Sep 24, 2019
@campoy
Copy link
Contributor

campoy commented Sep 24, 2019

In order to clarify the example, let's change the name of the variable to avoid confusion with the count function itself.

{
  people as personCount(func:type(Person)) {
    total: count(uid)
  }
  
  allPerson(func:uid(people)) {
    name
  }
}

This should return

{
  "data": {
    "personCount": {
      "total": 3,
    },
    "allPerson": [
      {
        "name": "Michael"
      },
      {
        "name": "Catalina"
      },
      {
        "name": "Sarah"
      }
    ]
  },
  "extensions": { ... }
}

This doesn't currently work, because the variable people doesn't seem to be defined.

On the other hand, if we comment out the total: count(uid) line in the first query fragment, the result is correct:

{
  people as personCount(func:type(Person)) {
    # total: count(uid)
  }
  
  allPerson(func:uid(people)) {
    name
  }
}
 Copy Text to Clipboard
{
  "data": {
    "personCount": [],
    "allPerson": [
      {
        "name": "Michael"
      },
      {
        "name": "Catalina"
      },
      {
        "name": "Sarah"
      }
    ]
  },
  "extensions": { ... }
}

Not sure whether having multiple aliases to count(uid) or not is related to this, but let's focus on finding the root cause for the people variable not being declared when a count(uid) appears in it.

@mangalaman93
Copy link
Contributor

mangalaman93 commented Sep 24, 2019

The root cause for this issue is the line here https://github.com/dgraph-io/dgraph/blob/master/gql/parser.go#L2848

The multiple aliases to count(uid) is related to this issue. Seems like instead of creating a child SubGraph for total: count(uid), we instead just use the parent SubGraph to store the information in case when count(uid) is used in the SubGraph. I plan to talk to @pawanrawal about this tomorrow and figure out why we handle this case like this instead of creating a child SubGraph

This regression was introduced in the PR #2947

@campoy
Copy link
Contributor

campoy commented Sep 26, 2019

Hey @mangalaman93, do you have any updates on this issue?

@mangalaman93
Copy link
Contributor

I was working on another issue. I will work on a solution for this issue today. I already have a plan in mind to fix it.

@mangalaman93
Copy link
Contributor

I am still trying to figure out a clean fix for the issue. The current implementation is restrictive in some ways, including this bug and requires a more thorough investigation of the issue. I have understood the root cause of the issue and should have a fix soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/querylang Issues related to the query language specification and implementation. kind/bug Something is broken. priority/P0 Critical issue that requires immediate attention. status/accepted We accept to investigate/work on it.
Development

No branches or pull requests

4 participants