diff --git a/edgraph/nquads_from_json.go b/edgraph/nquads_from_json.go index bddcacfae8e..dc4c2c80bbe 100644 --- a/edgraph/nquads_from_json.go +++ b/edgraph/nquads_from_json.go @@ -88,7 +88,11 @@ func parseFacets(m map[string]interface{}, prefix string) ([]*api.Facet, error) } // convert facet val interface{} to binary - tid := facets.TypeIDFor(&api.Facet{ValType: f.ValType}) + tid, err := facets.TypeIDFor(&api.Facet{ValType: f.ValType}) + if err != nil { + return nil, err + } + fVal := &types.Val{Tid: types.BinaryID} if err := types.Marshal(types.Val{Tid: tid, Value: fv}, fVal); err != nil { return nil, err diff --git a/edgraph/server.go b/edgraph/server.go index 7562b2a794d..3cb878681c2 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -629,13 +629,11 @@ func parseMutationObject(mu *api.Mutation) (*gql.Mutation, error) { res.Del = append(res.Del, nqs...) } - // We check that the facet value is in the right format based on the facet type. - for _, m := range mu.Set { - for _, f := range m.Facets { - if err := facets.TryValFor(f); err != nil { - return nil, err - } - } + // parse facets and convert to the binary format so that + // a field of type datetime like "2017-01-01" can be correctly encoded in the + // marshaled binary format as done in the time.Marshal method + if err := validateAndConvertFacets(mu); err != nil { + return nil, err } res.Set = append(res.Set, mu.Set...) @@ -647,6 +645,27 @@ func parseMutationObject(mu *api.Mutation) (*gql.Mutation, error) { return res, nil } +func validateAndConvertFacets(mu *api.Mutation) error { + for _, m := range mu.Set { + encodedFacets := make([]*api.Facet, 0, len(m.Facets)) + for _, f := range m.Facets { + // try to interpret the value as binary first + if _, err := facets.ValFor(f); err == nil { + encodedFacets = append(encodedFacets, f) + } else { + encodedFacet, err := facets.FacetFor(f.Key, string(f.Value)) + if err != nil { + return err + } + encodedFacets = append(encodedFacets, encodedFacet) + } + } + + m.Facets = encodedFacets + } + return nil +} + func validateNQuads(set, del []*api.NQuad) error { for _, nq := range set { var ostar bool diff --git a/query/query.go b/query/query.go index 65c7e5b60a0..b98ca531978 100644 --- a/query/query.go +++ b/query/query.go @@ -447,7 +447,12 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { if pc.Params.Facet != nil && len(fcsList) > childIdx { fs := fcsList[childIdx] for _, f := range fs.Facets { - uc.AddValue(facetName(fieldName, f), facets.ValFor(f)) + fVal, err := facets.ValFor(f) + if err != nil { + return err + } + + uc.AddValue(facetName(fieldName, f), fVal) } } @@ -483,7 +488,12 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { if pc.Params.Facet != nil && len(pc.facetsMatrix[idx].FacetsList) > 0 { // in case of Value we have only one Facets for _, f := range pc.facetsMatrix[idx].FacetsList[0].Facets { - dst.AddValue(facetName(fieldName, f), facets.ValFor(f)) + fVal, err := facets.ValFor(f) + if err != nil { + return err + } + + dst.AddValue(facetName(fieldName, f), fVal) } } @@ -1495,11 +1505,20 @@ func (sg *SubGraph) populateFacetVars(doneVars map[string]varValue, sgPath []*Su fvar, ok := sg.Params.FacetVar[f.Key] if ok { if pVal, ok := doneVars[fvar].Vals[uid]; !ok { - doneVars[fvar].Vals[uid] = facets.ValFor(f) + fVal, err := facets.ValFor(f) + if err != nil { + return err + } + + doneVars[fvar].Vals[uid] = fVal } else { // If the value is int/float we add them up. Else we throw an error as // many to one maps are not allowed for other types. - nVal := facets.ValFor(f) + nVal, err := facets.ValFor(f) + if err != nil { + return err + } + if nVal.Tid != types.IntID && nVal.Tid != types.FloatID { return x.Errorf("Repeated id with non int/float value for facet var encountered.") } @@ -2210,7 +2229,12 @@ func (sg *SubGraph) sortAndPaginateUsingFacet(ctx context.Context) error { } } if facet != nil { - values = append(values, []types.Val{facets.ValFor(facet)}) + fVal, err := facets.ValFor(facet) + if err != nil { + return err + } + + values = append(values, []types.Val{fVal}) } else { values = append(values, []types.Val{{Value: nil}}) } diff --git a/query/shortest.go b/query/shortest.go index 2f04ddf2acf..79416b20a19 100644 --- a/query/shortest.go +++ b/query/shortest.go @@ -117,7 +117,10 @@ func (sg *SubGraph) getCost(matrix, list int) (cost float64, rerr = x.Errorf("Expected 1 but got %d facets", len(fcs.Facets)) return cost, fcs, rerr } - tv := facets.ValFor(fcs.Facets[0]) + tv, err := facets.ValFor(fcs.Facets[0]) + if err != nil { + return 0.0, nil, err + } if tv.Tid == types.IntID { cost = float64(tv.Value.(int64)) } else if tv.Tid == types.FloatID { diff --git a/systest/mutations_test.go b/systest/mutations_test.go index 0e539ec102b..3e314e23abe 100644 --- a/systest/mutations_test.go +++ b/systest/mutations_test.go @@ -255,8 +255,8 @@ func DeleteAllReverseIndex(t *testing.T, c *dgo.Dgraph) { Running a query would make sure that the previous mutation for creating the link has completed with a commitTs from zero, and the subsequent deletion is done *AFTER* the link creation. - */ - c.NewReadOnlyTxn().Query(ctx, fmt.Sprintf("{ q(func: uid(%s)) { link { uid } }}", aId)) + */ + c.NewReadOnlyTxn().Query(ctx, fmt.Sprintf("{ q(func: uid(%s)) { link { uid } }}", aId)) _, err = c.NewTxn().Mutate(ctx, &api.Mutation{ CommitNow: true, @@ -976,7 +976,7 @@ func DeleteWithExpandAll(t *testing.T, c *dgo.Dgraph) { require.Equal(t, 0, len(r.Me)) } -func FacetsUsingNQuadsError(t *testing.T, c *dgo.Dgraph) { +func testTimeValue(t *testing.T, c *dgo.Dgraph, timeBytes []byte) { nquads := []*api.NQuad{ &api.NQuad{ Subject: "0x01", @@ -985,7 +985,7 @@ func FacetsUsingNQuadsError(t *testing.T, c *dgo.Dgraph) { Facets: []*api.Facet{ { Key: "since", - Value: []byte(time.Now().Format(time.RFC3339)), + Value: timeBytes, ValType: api.Facet_DATETIME, }, }, @@ -994,14 +994,6 @@ func FacetsUsingNQuadsError(t *testing.T, c *dgo.Dgraph) { mu := &api.Mutation{Set: nquads, CommitNow: true} ctx := context.Background() _, err := c.NewTxn().Mutate(ctx, mu) - require.Error(t, err) - require.Contains(t, err.Error(), "Error while parsing facet") - - nquads[0].Facets[0].Value, err = time.Now().MarshalBinary() - require.NoError(t, err) - - mu = &api.Mutation{Set: nquads, CommitNow: true} - _, err = c.NewTxn().Mutate(context.Background(), mu) require.NoError(t, err) q := `query test($id: string) { @@ -1015,6 +1007,21 @@ func FacetsUsingNQuadsError(t *testing.T, c *dgo.Dgraph) { require.Contains(t, string(resp.Json), "since") } +func FacetsUsingNQuadsError(t *testing.T, c *dgo.Dgraph) { + // test time in go binary format + timeBinary, err := time.Now().MarshalBinary() + require.NoError(t, err) + testTimeValue(t, c, timeBinary) + + // test time in full RFC3339 string format + testTimeValue(t, c, []byte(time.Now().Format(time.RFC3339))) + + // test time in partial string formats + testTimeValue(t, c, []byte("2018")) + testTimeValue(t, c, []byte("2018-01")) + testTimeValue(t, c, []byte("2018-01-01")) +} + func SkipEmptyPLForHas(t *testing.T, c *dgo.Dgraph) { ctx := context.Background() _, err := c.NewTxn().Mutate(ctx, &api.Mutation{ diff --git a/types/facets/facet_types.go b/types/facets/facet_types.go index ba1a5b35432..6850599e429 100644 --- a/types/facets/facet_types.go +++ b/types/facets/facet_types.go @@ -44,20 +44,3 @@ func ValTypeForTypeID(typId TypeID) api.Facet_ValType { } panic("Unhandled case in ValTypeForTypeID.") } - -// TypeIDForValType gives TypeID for api.Facet_ValType -func TypeIDForValType(valType api.Facet_ValType) TypeID { - switch valType { - case api.Facet_INT: - return IntID - case api.Facet_FLOAT: - return FloatID - case api.Facet_BOOL: - return BoolID - case api.Facet_DATETIME: - return DateTimeID - case api.Facet_STRING: - return StringID - } - panic("Unhandled case in TypeIDForValType.") -} diff --git a/types/facets/utils.go b/types/facets/utils.go index 885aae5b48a..bb1287a6ec0 100644 --- a/types/facets/utils.go +++ b/types/facets/utils.go @@ -135,7 +135,11 @@ func FacetFor(key, val string) (*api.Facet, error) { } // convert facet val interface{} to binary - tid := TypeIDFor(&api.Facet{ValType: vt}) + tid, err := TypeIDFor(&api.Facet{ValType: vt}) + if err != nil { + return nil, err + } + fVal := &types.Val{Tid: types.BinaryID} if err = types.Marshal(types.Val{Tid: tid, Value: v}, fVal); err != nil { return nil, err @@ -174,38 +178,30 @@ func SameFacets(a []*api.Facet, b []*api.Facet) bool { } // TypeIDFor gives TypeID for facet. -func TypeIDFor(f *api.Facet) types.TypeID { - switch TypeIDForValType(f.ValType) { - case IntID: - return types.IntID - case StringID: - return types.StringID - case BoolID: - return types.BoolID - case DateTimeID: - return types.DateTimeID - case FloatID: - return types.FloatID +func TypeIDFor(f *api.Facet) (types.TypeID, error) { + switch f.ValType { + case api.Facet_INT: + return types.IntID, nil + case api.Facet_FLOAT: + return types.FloatID, nil + case api.Facet_BOOL: + return types.BoolID, nil + case api.Facet_DATETIME: + return types.DateTimeID, nil + case api.Facet_STRING: + return types.StringID, nil default: - panic("unhandled case in facetValToTypeVal") + return types.DefaultID, fmt.Errorf("Unrecognized facet type: %v", f.ValType) } } -// TryValFor tries to convert the facet to the its type from binary format. We use it to validate -// the facets set directly by the user during mutation. -func TryValFor(f *api.Facet) error { - val := types.Val{Tid: types.BinaryID, Value: f.Value} - typId := TypeIDFor(f) - _, err := types.Convert(val, typId) - return x.Wrapf(err, "Error while parsing facet: [%v]", f) -} - // ValFor converts Facet into types.Val. -func ValFor(f *api.Facet) types.Val { +func ValFor(f *api.Facet) (types.Val, error) { val := types.Val{Tid: types.BinaryID, Value: f.Value} - typId := TypeIDFor(f) - v, err := types.Convert(val, typId) - x.AssertTruef(err == nil, - "We should always be able to covert facet into val. %v %v", f.Value, typId) - return v + facetTid, err := TypeIDFor(f) + if err != nil { + return types.Val{}, err + } + + return types.Convert(val, facetTid) } diff --git a/worker/export.go b/worker/export.go index 337fac76917..e2c5c608e64 100644 --- a/worker/export.go +++ b/worker/export.go @@ -101,12 +101,29 @@ func toRDF(pl *posting.List, prefix string, readTs uint64) (*pb.KV, error) { } buf.WriteString(f.Key) buf.WriteByte('=') - fVal := &types.Val{Tid: types.StringID} - x.Check(types.Marshal(facets.ValFor(f), fVal)) - if facets.TypeIDFor(f) == types.StringID { - buf.WriteString(strconv.Quote(fVal.Value.(string))) + + fVal, err := facets.ValFor(f) + if err != nil { + glog.Errorf("Error getting value from facet %#v:%v", f, err) + continue + } + + fStringVal := &types.Val{Tid: types.StringID} + if err = types.Marshal(fVal, fStringVal); err != nil { + glog.Errorf("Error while marshaling facet value %v to string: %v", + fVal, err) + continue + } + facetTid, err := facets.TypeIDFor(f) + if err != nil { + glog.Errorf("Error getting type id from facet %#v:%v", f, err) + continue + } + + if facetTid == types.StringID { + buf.WriteString(strconv.Quote(fStringVal.Value.(string))) } else { - buf.WriteString(fVal.Value.(string)) + buf.WriteString(fStringVal.Value.(string)) } } buf.WriteByte(')') diff --git a/worker/task.go b/worker/task.go index 959bfd08e81..c81578ea65e 100644 --- a/worker/task.go +++ b/worker/task.go @@ -1502,7 +1502,11 @@ func applyFacetsTree(postingFacets []*api.Facet, ftree *facetsTree) (bool, error switch fnType { case CompareAttrFn: // lt, gt, le, ge, eq var err error - typId := facets.TypeIDFor(fc) + typId, err := facets.TypeIDFor(fc) + if err != nil { + return false, err + } + v, has := ftree.function.convertedVal[typId] if !has { if v, err = types.Convert(ftree.function.val, typId); err != nil { @@ -1512,10 +1516,19 @@ func applyFacetsTree(postingFacets []*api.Facet, ftree *facetsTree) (bool, error ftree.function.convertedVal[typId] = v } } - return types.CompareVals(fname, facets.ValFor(fc), v), nil + fVal, err := facets.ValFor(fc) + if err != nil { + return false, err + } + + return types.CompareVals(fname, fVal, v), nil case StandardFn: // allofterms, anyofterms - if facets.TypeIDForValType(fc.ValType) != facets.StringID { + facetType, err := facets.TypeIDFor(fc) + if err != nil { + return false, err + } + if facetType != types.StringID { return false, nil } return filterOnStandardFn(fname, fc.Tokens, ftree.function.tokens)