{"action":"created","issue":{"url":"https://api.github.com/repos/cayleygraph/cayley/issues/378","repository_url":"https://api.github.com/repos/cayleygraph/cayley","labels_url":"https://api.github.com/repos/cayleygraph/cayley/issues/378/labels{/name}","comments_url":"https://api.github.com/repos/cayleygraph/cayley/issues/378/comments","events_url":"https://api.github.com/repos/cayleygraph/cayley/issues/378/events","html_url":"https://github.com/cayleygraph/cayley/pull/378","id":136986622,"number":378,"title":"Typed Quad values","user":{"login":"dennwc","id":676724,"avatar_url":"https://avatars.githubusercontent.com/u/676724?v=3","gravatar_id":"","url":"https://api.github.com/users/dennwc","html_url":"https://github.com/dennwc","followers_url":"https://api.github.com/users/dennwc/followers","following_url":"https://api.github.com/users/dennwc/following{/other_user}","gists_url":"https://api.github.com/users/dennwc/gists{/gist_id}","starred_url":"https://api.github.com/users/dennwc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dennwc/subscriptions","organizations_url":"https://api.github.com/users/dennwc/orgs","repos_url":"https://api.github.com/users/dennwc/repos","events_url":"https://api.github.com/users/dennwc/events{/privacy}","received_events_url":"https://api.github.com/users/dennwc/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-02-28T00:35:15Z","updated_at":"2016-07-16T22:04:38Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cayleygraph/cayley/pulls/378","html_url":"https://github.com/cayleygraph/cayley/pull/378","diff_url":"https://github.com/cayleygraph/cayley/pull/378.diff","patch_url":"https://github.com/cayleygraph/cayley/pull/378.patch"},"body":"### Strictly-typed quad values\r\n\r\nThe idea is to change string type of quad directions to interface-based Value, to allow to preserve types of subject and object particularly.\r\n\r\n```Go\r\ntype Value interface {\r\n\tString() string\r\n}\r\n\r\ntype Quad struct {\r\n\tSubject   Value\r\n\tPredicate Value\r\n\tObject    Value\r\n\tLabel     Value\r\n}\r\n```\r\n\r\nAs a part of effort, unify tests for QuadStore to check that all implementations behaves in a similar way.\r\n\r\n### Supported types:\r\n```Go\r\n// Raw is a Turtle/NQuads-encoded value.\r\ntype Raw string\r\n\r\n// String is an RDF string value (ex: \"name\").\r\ntype String string\r\n\r\n// TypedString is an RDF value with type (ex: \"name\"^^<type>).\r\ntype TypedString struct {\r\n\tValue String\r\n\tType  IRI\r\n}\r\n\r\n// LangString is an RDF string with language (ex: \"name\"@lang).\r\ntype LangString struct {\r\n\tValue String\r\n\tLang  string\r\n}\r\n\r\n// IRI is an RDF Internationalized Resource Identifier (ex: <name>).\r\ntype IRI string\r\n\r\n// BNode is an RDF Blank Node (ex: _:name).\r\ntype BNode string\r\n\r\n// Int is a native wrapper for int64 type.\r\ntype Int int64\r\n\r\n// Float is a native wrapper for float64 type.\r\ntype Float float64\r\n\r\n// Bool is a native wrapper for bool type.\r\ntype Bool bool\r\n\r\n// Time is a native wrapper for time.Time type.\r\ntype Time time.Time\r\n```\r\n### TODO:\r\n- [x] Value definition\r\n- [x] Update cquads and nquads parsers and tests\r\n- [x] Update QS and tests\r\n- [x] Update integration tests\r\n- [x] Unified tests for QS\r\n- [x] Memstore support\r\n- [x] Bolt support\r\n- [x] LevelDB support (switch to protobuf)\r\n- [x] Mongo support (untyped)\r\n- [x] SQL support (untyped)\r\n- [x] GAE support\r\n- [x] Support for native types (int, float, etc)\r\n- [x] Typed values in Mongo\r\n- [x] Typed values in SQL\r\n- [ ] Typed values in GAE\r\n- [x] Path lib support\r\n- [x] Gremlin support\r\n- [x] Recognize TypedString and LandString in cquads parser\r\n- [x] Registration for TypedString converters\r\n- [ ] Support for custom types\r\n- [ ] Migration code for SQL\r\n\r\n#### Problems so far:\r\n- [ ] goprotobuf will not work on GAE, thus we have no support for typed values for now\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/cayleygraph/cayley/378)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/cayleygraph/cayley/issues/comments/233153377","html_url":"https://github.com/cayleygraph/cayley/pull/378#issuecomment-233153377","issue_url":"https://api.github.com/repos/cayleygraph/cayley/issues/378","id":233153377,"user":{"login":"barakmich","id":98356,"avatar_url":"https://avatars.githubusercontent.com/u/98356?v=3","gravatar_id":"","url":"https://api.github.com/users/barakmich","html_url":"https://github.com/barakmich","followers_url":"https://api.github.com/users/barakmich/followers","following_url":"https://api.github.com/users/barakmich/following{/other_user}","gists_url":"https://api.github.com/users/barakmich/gists{/gist_id}","starred_url":"https://api.github.com/users/barakmich/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/barakmich/subscriptions","organizations_url":"https://api.github.com/users/barakmich/orgs","repos_url":"https://api.github.com/users/barakmich/repos","events_url":"https://api.github.com/users/barakmich/events{/privacy}","received_events_url":"https://api.github.com/users/barakmich/received_events","type":"User","site_admin":false},"created_at":"2016-07-16T22:04:38Z","updated_at":"2016-07-16T22:04:38Z","body":"We're almost there. With the corresponding export for JS strings (in comments) and the updated tests to match, I think we can merge it!\n\n---\n\nReviewed 8 of 8 files at r4.\nReview status: all files reviewed at latest revision, 3 unresolved discussions.\n\n---\n\n*[query/gremlin/environ.go, line 234 \\[r4\\]](https://reviewable.io:443/reviews/cayleygraph/cayley/378#-KMphVEYlfQ4dcEpxd60:-KMphVEYlfQ4dcEpxd61:-788682789) ([raw file](https://github.com/cayleygraph/cayley/blob/0508015d04d590a9b6611223bdd4c21feb835304/query/gremlin/environ.go#L234)):*\n> ```Go\n> \tcase string:\n> \t\tif len(v) > 2 {\n> \t\t\tif v[0] == '<' && v[len(v)-1] == '>' {\n> ```\n\nMaybe this could be a function in quad? `quad.FromJS(v)`? Because...\n\n---\n\n*[query/gremlin/session.go, line 222 \\[r4\\]](https://reviewable.io:443/reviews/cayleygraph/cayley/378#-KMphrkerdN-frVPBUzH:-KMphrkerdN-frVPBUzI:-574836393) ([raw file](https://github.com/cayleygraph/cayley/blob/0508015d04d590a9b6611223bdd4c21feb835304/query/gremlin/session.go#L222)):*\n> ```Go\n> \t\t\tfor _, k := range tagKeys {\n> \t\t\t\tif name := s.qs.NameOf(tags[k]); name != nil {\n> \t\t\t\t\tobj[k] = name.String()\n> ```\n\nHere we can have name.ToJS() -- and remove the extraneous quotes on the returned values.\n\nThis is hugely important for things like\n\n```\n<v> <foo> \"node\"\n<v> <name> \"v\"\n```\n\n```\ns = g.V(\"node\").In(\"<foo>\").Out(\"<name>\").ToValue\ng.V(s) // s is '\"v\"' not 'v', and this query won't match the right node\n```\n\nIn short, if we accept it, we have to return it under the same rules.\n\nFortunately, that function is easy. It's just:\n* If we're of type quad.String\n  * return internal string\n* else return quad.String(), as this line does.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cayleygraph/cayley/378#-:-KMpj7uYXE_J_B7XGNuJ:788313866)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"barakmich"}}