Bug with multi-level (nested bocks) in Upsert Block

[MichelDiz]

What version of Dgraph are you using?

Latest

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, OS)?

N/A

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

Mutation Sample

I added "dgraph.type" just to be sure.

{
  "set": [
    {
      "type": "node",
      "dgraph.type": "Node",
      "id": "0",
      "labels": ["Movie"],
      "properties": {
        "tagline": "Welcome to the Real World",
        "title": "The Matrix",
        "released": 1999,
        "dgraph.type": "Properties"
      }
    }
]
}

type Node {
  properties
  type
  id
  labels
  tagline
  title
  released
}

type Properties {
  tagline
  title
  released
}

Query example

{  
  movies(func: has(properties)) @filter(eq(type,"node")){
    type
    id
    labels
    dgraph.type
    properties {
      tagline
      title
      released
    }
      tagline
      title
      released
  }
}

Run the Upsert Block

upsert {
  query {
    movies(func: has(type)) @filter(has(properties)){
      v as uid
      Labels as labels
      properties {
        Title as title
        Released as released
        Tagline as tagline
      }
    }
  }
  mutation {
    set {
      uid(v) <migrationDone> "True" .
      uid(v) <title> val(Title) .
      uid(v) <dgraph.type> val(Labels) .
      uid(v) <released> val(Released) .
      uid(v) <tagline> val(Tagline) .
    }
  }
}

Expected behaviour and actual result.

This operation should be able to copy the values ​​in the variables of the nested block, "properties". Thus allowing a kind of "migration" using Upsert Block. This feature is also equivalent to "merge". Where we collect the values ​​of the nested block, we paste in the target node and delete the nested block right after.

With this "Hack" I was able to make it work

WARNING - This "hack" works mutation by mutation. Not Bulk Upsert. If you try to bulk upsert, it can get crazy results.

upsert {
  query {
    movies(func: has(type)) @filter(has(properties)){
      v as uid
      Labels as labels
      properties {
        Title as title
        Released as released
        Tagline as tagline
      }
    }
 me() {
    TitleMe as sum(val(Title))
    ReleasedMe as sum(val(Released))
    TaglineMe as sum(val(Tagline))
  }
  }
  mutation {
    set {
      uid(v) <migrationDone> "True" .
      uid(v) <title> val(TitleMe) .
      uid(v) <dgraph.type> val(Labels) .
      uid(v) <released> val(ReleasedMe) .
      uid(v) <tagline> val(TaglineMe) .
    }
  }
}

Just an Update

Testing the PR #4767 for this issue - but now creating a new node instead of update it (which is the goal of 4692, to just update instead of creating new ones). It works like that.

pay attention that this would work only with 1 to 1 operation. It won't work in 1 to many (like bulk upsert).

upsert {
  query {
    movies(func: has(type), first:1) @filter(has(properties) AND not has(migrationDone)){
     uid
      qID as id
      Labels as labels
      properties {
        Title as title
        Released as released
        Tagline as tagline
      }
    }
 me() {
    LabelsMe as sum(val(Labels))
    TitleMe as sum(val(Title))
    ReleasedMe as sum(val(Released))
    TaglineMe as sum(val(Tagline))
    qIDMe as sum(val(qID))
  }
  }
  mutation {
    set {
      _:New <migrationDone> "True" .
      _:New <id> val(qIDMe) .
      _:New <title> val(TitleMe) .
      _:New <dgraph.type> val(LabelsMe) .
      _:New <released> val(ReleasedMe) .
      _:New <tagline> val(TaglineMe) .
    }
  }
}

[mangalaman93]

Further work that needs to be done:

• How would SQL handle this case?
• Is it possible to do this with existing functionality at all?

Blocks #4767

[MichelDiz]

In SQL it has no exact similarity cuz it is not a Graph DB. But in theory, it would be the same as MERGE https://docs.microsoft.com/en-us/sql/t-sql/statements/merge-transact-sql?view=sql-server-ver15 - Which is the ability to merge two tables in one. Which would be the case with this operation that I'm using Upsert Block.

In the above operation, I am merging two nodes. "Parent" with "Child", that is, bringing the nested data to the parent. And then I would delete the nested data.

Yes, it is possible to do with the existing functionality. But it is limited. It is necessary to create an extra block in a kind of "Hacky way" for it to work properly. However, sometimes it doesn't work as expected. Also, bulk upsert is impossible with this approach. The user needs to spend hundreds or thousands of Upsert Blocks to complete the entire process.

BTW, this issue is slightly different from #4767 the only thing that is similar is the use of an aggregation block to make this feature work.

[MichelDiz]

Adding a use case based on a question did by a user in Slack.

Question:

Does anyone have an experience loading big datasets into a live cluster.
The issue we're facing is to load a large amount of vertexes and edges into the cluster, but some of these nodes may already exist and we dont want duplicates.
I was hoping to be able to write all the data as files with an array of upsert blocks, and feed it to the live loader, but that does not seem to work.
Any ideas?

What I would answer.

Upsert Block is a too new feature. It won’t work in any loader.
One thing you can do is add a unique edge with a unique value for each entity. And then, when it is totally loaded. Do upserts to merge them.

But I just remembered that this isn't possible either. Due to these issues.

[mangalaman93]

I was looking around as to how upsert works in other databases. I didn't find any example where a database would allow updates using the values of variables defined in the query of the upsert.

I also noticed that databases only allow single row update or same update in all the rows that satisfy the query criteria.

I recommend that we keep the current behaviour for val as it is because it provides one to one mapping of nodes to their values which is similar to single row update that other databases provide. I also think that we should accept the change proposed in the PR #4767 to at least allow some of the use cases.

We can keep thinking how we can better solve the problems raised by this issue. I do not have a good solution that seems worth implementing.

cc @manishrjain @MichelDiz

[seanlaff]

@mangalaman93 Perhaps this is a new feature unrelated to variables? What if there was a function to return a predicate's value from a given uid? Intuitively, if I can say uid(v) in a mutation, I'd expect to also be able to say something like predicateValue(uid(v), "starttime"). I think this would solve my issues, what about you @MichelDiz ?

As a small note, dgraph does not error when you try to use vars like this in bulk contexts, which is probably a small bug given it is not currently unsupported, e.x the _:newNode <endtime> val(endVar) . line below doesn't do anything

upsert {
  query {
    v AS var(func: eq("eventtype", "myevent")) @filter(lt(starttime, "2005") AND gt(endtime, "2005")) {
        startVar as starttime
        endVar as endtime
    }
  }
  mutation @if(gt(len(v), 0)) {
    set {
      uid(v) <endtime> "2005" .
      _:newNode <starttime> "2005" .
      _:newNode <endtime> val(endVar) .
    }
  }
}

I'm using dgraph to store event information, nodes look like this { eventType, start, end }.

My api receives discrete beginEvent and finishEvent events from a message bus (which happens to not guarantee ordering)

When I ingest to dgraph, I want to do conditional upserts- for example:
If I receive a beginEvent at t2, I'd like to find all events that span that time, for example an event like {eventType: myevent, start: t0, end: t5} and using an upsert... end up with two nodes {start: t0, end:t2} and {start: t2, end:t5}. (With an approach like this, I'll have eventual consistency even if my events are delivered out of order)

The bug fix that recently went in makes this possible for a single event update at a time (using that me(){ myvar as sum()} syntax), but I'd like to be able to do batches for performance reasons.

[mangalaman93]

@seanlaff This seems like a good idea. Let me think through this.

[mangalaman93]

I just realized that what you are suggesting is already possible. You could do something like:

upsert {
 query {
  .
  .
  . 

  var(func: uid(v)) {
    p as  starttime
  }
 }

 # Now you can use val(p) along with uid(v) and it'd work fine.
 # something like below
 mutation {
  set {
   uid(v) <SomePredicate> val(p) .
  }
 }
}

Now, the problem we are trying to solve is when different set of UIDs needs to be correlated UIDs in a variable.

[seanlaff]

@mangalaman93 Exactly, it's possible with one match but does not work with an array of matches. I was thinking perhaps a new func since the mutation doesn't exist within the closure of the block in which predicate var was defined... but on the other hand, my first intuition when writing the query was that I could use a predicate var within the mutation and the planner would "know" which uid to pull it from- so perhaps that is a better goal to strive for.