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

feat(dgraph): Add suport for RDF query. #6038

Merged
merged 16 commits into from
Aug 7, 2020
Merged

feat(dgraph): Add suport for RDF query. #6038

merged 16 commits into from
Aug 7, 2020

Conversation

poonai
Copy link
Contributor

@poonai poonai commented Jul 20, 2020

This PR will allow users to get the query results in RDF format.
Implementation Details: https://discuss.dgraph.io/t/rfc-streaming-rdf-queries/8410


This change is Reviewable

Docs Preview: Dgraph Preview

பாலாஜி added 4 commits July 20, 2020 16:34
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
பாலாஜி added 2 commits July 20, 2020 16:47
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
go.mod Outdated
@@ -17,7 +17,7 @@ require (
github.com/blevesearch/snowballstem v0.0.0-20180110192139-26b06a2c243d // indirect
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd
github.com/dgraph-io/badger/v2 v2.0.1-rc1.0.20200715005050-3ffaf3cd1d4a
github.com/dgraph-io/dgo/v200 v200.0.0-20200401175452-e463f9234453
github.com/dgraph-io/dgo/v200 v200.0.0-20200720091212-f41d047addd1
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove these changes from the merge, 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.

Yes

Copy link
Contributor

@MichelDiz MichelDiz left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @balajijinnah, @manishrjain, @parasssh, @pawanrawal, and @vvbalaji-dgraph)


query/rdf_result_test.go, line 40 at r1 (raw file):

	require.NoError(t, err)
	require.Equal(t, string(rdf), `<0x1> <name> "Michonne" 
<0x1> <friend> <0x17> 

For some reason, I'm not seeing the dots at the end of each RDF line. Is it right?

Signed-off-by: பாலாஜி <balaji@dgraph.io>
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Address my comments, please.

Reviewed 4 of 6 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @anurags92, @ashish-goswami, @balajijinnah, @parasssh, @pawanrawal, and @vvbalaji-dgraph)


query/outputrdf.go, line 15 at r2 (raw file):

 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

Need a space here.


query/outputrdf.go, line 29 at r2 (raw file):

// ToRDF converts the given subgraph list into rdf format.
func ToRDF(l *Latency, sgl []*SubGraph) ([]byte, error) {
	output := ""

Use a bytes.Buffer or something.


query/outputrdf.go, line 37 at r2 (raw file):

			}
		}

vertical space.


query/outputrdf.go, line 39 at r2 (raw file):

	}
	return []byte(output), nil

Avoid this allocation.


query/outputrdf.go, line 44 at r2 (raw file):

// castToRDF converts the given subgraph to RDF and appends to the
// output string.
func castToRDF(output *string, sg *SubGraph) error {

I'd convert all these funcs into methods. And the type struct can hold the bytes.Buffer.


query/outputrdf.go, line 45 at r2 (raw file):

// output string.
func castToRDF(output *string, sg *SubGraph) error {

Vertical space.


query/outputrdf.go, line 50 at r2 (raw file):

		return errors.New("uid count is not supported in the rdf output format")
	}

here.


query/outputrdf.go, line 72 at r2 (raw file):

	// Recursively cnvert RDF for the children graph.
	for _, child := range sg.Children {
		err := castToRDF(output, child)

if err := ; err != nil { ... }


query/outputrdf.go, line 102 at r2 (raw file):

				return err
			}
			*output += fmt.Sprintf("<%#x> <%s> %s .\n", uid, sg.aggWithVarFieldName(),

fmt.Sprintf is very expensive.

Use bytes.Buffer.


query/outputrdf.go, line 103 at r2 (raw file):

			}
			*output += fmt.Sprintf("<%#x> <%s> %s .\n", uid, sg.aggWithVarFieldName(),
				string(outputval))

buf.Write(outputval)


query/outputrdf.go, line 128 at r2 (raw file):

		fieldName = fmt.Sprintf("count(%s)", sg.Attr)
	}
	*output += fmt.Sprintf("<%#x> <%s> %d .\n", subject, fieldName, count)

small helper methods, which you can give the subject, pred, object to, and it would just write it to buf.

பாலாஜி added 5 commits July 29, 2020 16:10
Signed-off-by: பாலாஜி <balaji@dgraph.io>
…aji/streaming_rdf

Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Copy link
Contributor Author

@poonai poonai left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 6 files reviewed, 13 unresolved discussions (waiting on @anurags92, @ashish-goswami, @balajijinnah, @manishrjain, @MichelDiz, @parasssh, @pawanrawal, and @vvbalaji-dgraph)


query/outputrdf.go, line 15 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Need a space here.

Done.


query/outputrdf.go, line 29 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Use a bytes.Buffer or something.

Done.


query/outputrdf.go, line 39 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Avoid this allocation.

Done.


query/outputrdf.go, line 44 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

I'd convert all these funcs into methods. And the type struct can hold the bytes.Buffer.

Done.


query/outputrdf.go, line 45 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Vertical space.

Done.


query/outputrdf.go, line 50 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

here.

Done.


query/outputrdf.go, line 72 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

if err := ; err != nil { ... }

Done.


query/outputrdf.go, line 102 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

fmt.Sprintf is very expensive.

Use bytes.Buffer.

Done.


query/outputrdf.go, line 103 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

buf.Write(outputval)

Done.


query/outputrdf.go, line 128 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

small helper methods, which you can give the subject, pred, object to, and it would just write it to buf.

Done.


query/rdf_result_test.go, line 40 at r1 (raw file):

Previously, MichelDiz (Michel Diz) wrote…

For some reason, I'm not seeing the dots at the end of each RDF line. Is it right?

Done.

Signed-off-by: பாலாஜி <balaji@dgraph.io>
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

:lgtm: after the bug is fixed

Reviewable status: 1 of 6 files reviewed, 15 unresolved discussions (waiting on @anurags92, @ashish-goswami, @balajijinnah, @manishrjain, @MichelDiz, @parasssh, and @vvbalaji-dgraph)


query/common_test.go, line 78 at r3 (raw file):

	res, err := txn.Do(ctx, &api.Request{
		Query:     query,
		RdfFormat: true,

We could call this as RespFormat and the value can be rdf. Default is json if nothing is given.


query/rdf_result_test.go, line 46 at r3 (raw file):

<0x18> <name> "Glenn Rhee" .
<0x19> <name> "Daryl Dixon" .
<0x17> <age> 15 .

"15"
All the values have to be string literal. Just verify other types also like boolean, float and datetime.

பாலாஜி added 3 commits August 5, 2020 10:19
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
Signed-off-by: பாலாஜி <balaji@dgraph.io>
@poonai poonai merged commit 898a8f3 into master Aug 7, 2020
@poonai poonai deleted the balaji/streaming_rdf branch August 7, 2020 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants