impl/memory: Fix time serialization encoding.

impl/memory/datastore_query.go

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package memory
 
 import (
         "bytes"
         "encoding/base64"
         "errors"
         "fmt"
+        "time"
 
         ds "github.com/luci/gae/service/datastore"
         "github.com/luci/gae/service/datastore/serialize"
         "github.com/luci/luci-go/common/cmpbin"
         "github.com/luci/luci-go/common/stringset"
 )
 
 // MaxQueryComponents was lifted from a hard-coded constant in dev_appserver.
 // No idea if it's a real limit or just a convenience in the current dev
 // appserver implementation.
@@ skipping 73 matching lines @@
         }
         if p, _, _ := fq.IneqFilterHigh(); p != "" {
                 numComponents++
         }
         for _, v := range fq.EqFilters() {
                 numComponents += v.Len()
         }
         return numComponents
 }
 
+// convertQueryProperty converts a ds.Property into a suitable property for
+// query serialization.
+//
+// PTTime fields are serialized into indexes as PTInt, so we need to represent
+// them as PTInt in the serialized query.

[iannucci, 2015/12/29 19:53:46]

could you add another case for all the various byte-type things (blobstore keys,
[]byte, string) all get the same collapsing behavior?

[dnj, 2015/12/29 21:40:47]

Sure. AFAICT this only additionally affected blobstore keys.

Since the logic is implemented in serialize, I moved this function into
serialize as well.

+func convertQueryProperty(p ds.Property) ds.Property {
+        switch p.Type() {
+        case ds.PTTime:
+                return ds.MkProperty(serialize.TimeToInt(p.Value().(time.Time)))
+        default:
+                return p
+        }
+}
+
 // GetBinaryBounds gets the binary encoding of the upper and lower bounds of

[iannucci, 2015/12/29 19:53:46]

this bug probably also affects equality comparisons. See line 198

[dnj, 2015/12/29 21:40:47]

yarp, nice catch. Added test case and fix.

 // the inequality filter on fq, if any is defined. If a bound does not exist,
 // it is nil.
 //
 // NOTE: if fq specifies a descending sort order for the inequality, the bounds
-// will be inverted, incremented, and fliped.
+// will be inverted, incremented, and flipped.
 func GetBinaryBounds(fq *ds.FinalizedQuery) (lower, upper []byte) {
         // Pick up the start/end range from the inequalities, if any.
         //
         // start and end in the reducedQuery are normalized so that `start >=
         // X < end`. Because of that, we need to tweak the inequality filters
         // contained in the query if they use the > or <= operators.
         if ineqProp := fq.IneqFilterProp(); ineqProp != "" {
                 _, startOp, startV := fq.IneqFilterLow()
                 if startOp != "" {
-                        lower = serialize.ToBytes(startV)
+                        lower = serialize.ToBytes(convertQueryProperty(startV))
                         if startOp == ">" {
                                 lower = increment(lower)
                         }
                 }
 
                 _, endOp, endV := fq.IneqFilterHigh()
                 if endOp != "" {
-                        upper = serialize.ToBytes(endV)
+                        upper = serialize.ToBytes(convertQueryProperty(endV))
                         if endOp == "<=" {
                                 upper = increment(upper)
                         }
                 }
 
                 // The inequality is specified in natural (ascending) order in the query's
                 // Filter syntax, but the order information may indicate to use a descending
                 // index column for it. If that's the case, then we must invert, swap and
                 // increment the inequality endpoints.
                 //
@@ skipping 131 matching lines @@
                 // and the new value has overflowed this representation.
                 //
                 // Fortunately, the first byte of a serialized index column entry is a
                 // PropertyType byte, and the only valid values that we'll be incrementing
                 // are never equal to 0xFF, since they have the high bit set (so either they're
                 // 0x8*, or 0x7*, depending on if it's inverted).
                 impossible(fmt.Errorf("incrementing %v would require more sigfigs", bstr))
         }
         return ret
 }