Skip to content

Conversation

@apr94
Copy link
Contributor

@apr94 apr94 commented May 17, 2017

I want to make sure that the addition I am making to the Statement struct looks good.

@apr94 apr94 force-pushed the construct_pattern branch from 95bdee8 to 57c192c Compare May 17, 2017 23:31
Copy link
Member

@xllora xllora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks just right on track. I just left a couple of comments. To make sure I read it correctly.

data []*triple.Triple
pattern []*GraphClause
workingClause *GraphClause
constructClauses []*ConstructClause
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate why you need a new working clause? The where clause of a construct is the same as the one on a regular select. I was wondering if it is just cleaner to just add the actual construct information containing the info to create the new triples. Or am I just mislead by the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So constructClauses captures only the declarations within a CONSTRUCT statement that specifies how the triples must be built. For instance, consider the following:

construct {?s "new_predicate"@[] ?o} into ?a from ?b where {?s "old_predicate"@[] ?o}

constructClauses simply captures the ?s "new_predicate"@[] ?o part. workingClauses will capture the ?s "old_predicate"@[] ?o as before. constructClauses will capture multiple such declarations (separated by the . operator) and constructClause also has support for multiple reification clauses. So in a way, constructClause is similar to data within the Statement struct (except it has to support literals, bindings and reification). Does this seem like the right thing to do?

}

// ConstructClause represents a singular clause within a construct statement.
type ConstructClause struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite the name this is just the info on the construct right? Just want to make sure I got it right :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it is just the information required to insert a new triple into the graph. For instance (a contrived example):

construct {?s ?p ?o; ?p1 ?o1} into ?a from ?b where {?s ?p ?o. ?s ?p1 ?o2}

will lead to the creation of the following instance of constructClauses:

[]*ConstructClauses{
  {
    SBinding: ?s,
    PBinding: ?p,
    OBinding: ?o,
    ReificationClauses: []*ReificationClause{
      {
         PBinding: ?p1
         OBinding: ?o1
      },
   },
}

I guess the name is confusing. Do you have any suggestions?

@xllora xllora merged commit f778287 into google:master May 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants