Fix bugs in service/datastore

service/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 datastore
 
 import (
         "bytes"
         "fmt"
         "sort"
@@ skipping 452 matching lines @@
 }
 
 // Finalize converts this Query to a FinalizedQuery. If the Query has any
 // inconsistencies or violates any of the query rules, that will be returned
 // here.
 func (q *Query) Finalize() (*FinalizedQuery, error) {
         if q.err != nil || q.finalized != nil {
                 return q.finalized, q.err
         }
 
+        ancestor := (*Key)(nil)
+        if slice, ok := q.eqFilts["__ancestor__"]; ok {
+                ancestor = slice[0].Value().(*Key)
+        }
+
         err := func() error {
+
                 if q.kind == "" { // kindless query checks
                         if q.ineqFiltProp != "" && q.ineqFiltProp != "__key__" {
                                 return fmt.Errorf(
                                         "kindless queries can only filter on __key__, got %q", q.ineqFiltProp)
                         }
-                        if len(q.eqFilts) > 0 {
-                                return fmt.Errorf("kindless queries not have any equality filters")
+                        allowedEqs := 0

[dnj, 2015/09/29 19:06:23]

nit: IMO just split this out into a "with ancestor" and "without ancestor" case,
each of which has its own more descriptive error message.

[iannucci, 2015/09/29 19:19:55]

but there's only one case. We're computing the threshold for comparison here.

+                        if ancestor != nil {
+                                allowedEqs = 1
+                        }
+                        if len(q.eqFilts) > allowedEqs {
+                                return fmt.Errorf("kindless queries may not have any equality filters")
                         }
                         for _, o := range q.order {
                                 if o.Property != "__key__" || o.Descending {
                                         return fmt.Errorf("invalid order for kindless query: %#v", o)
                                 }
                         }
                 }
 
                 if q.keysOnly && q.project != nil && q.project.Len() > 0 {
                         return errors.New("cannot project a keysOnly query")
                 }
 
                 if q.ineqFiltProp != "" {
                         if len(q.order) > 0 && q.order[0].Property != q.ineqFiltProp {
                                 return fmt.Errorf(
                                         "first sort order must match inequality filter: %q v %q",
                                         q.order[0].Property, q.ineqFiltProp)
                         }
                         if q.ineqFiltLowSet && q.ineqFiltHighSet {
-                                if q.ineqFiltHigh.Less(&q.ineqFiltLow) &&
+                                if q.ineqFiltHigh.Less(&q.ineqFiltLow) ||
                                         (q.ineqFiltHigh.Equal(&q.ineqFiltLow) &&
                                                 (!q.ineqFiltLowIncl || !q.ineqFiltHighIncl)) {
                                         return ErrNullQuery
                                 }
                         }
+                        if q.ineqFiltProp == "__key__" {
+                                if q.ineqFiltLowSet {
+                                        if ancestor != nil && !q.ineqFiltLow.Value().(*Key).HasAncestor(ancestor) {
+                                                return fmt.Errorf(
+                                                        "inequality filters on __key__ must be descendants of the __ancestor__")
+                                        }
+                                }
+                                if q.ineqFiltHighSet {
+                                        if ancestor != nil && !q.ineqFiltHigh.Value().(*Key).HasAncestor(ancestor) {
+                                                return fmt.Errorf(
+                                                        "inequality filters on __key__ must be descendants of the __ancestor__")
+                                        }
+                                }
+                        }
                 }
 
                 err := error(nil)
                 if q.project != nil {
                         q.project.Iter(func(p string) bool {
                                 if _, iseq := q.eqFilts[p]; iseq {
                                         err = fmt.Errorf("cannot project on equality filter field: %s", p)
                                         return false
                                 }
                                 return true
                         })
                 }
                 return err
         }()
         if err != nil {
                 q.err = err
                 return nil, err
         }
 
         ret := &FinalizedQuery{
                 original: q,
                 kind:     q.kind,
 
                 keysOnly:             q.keysOnly,
-                eventuallyConsistent: q.eventualConsistency || q.eqFilts["__ancestor__"] == nil,
+                eventuallyConsistent: q.eventualConsistency || ancestor == nil,
                 limit:                q.limit,
                 offset:               q.offset,
                 start:                q.start,
                 end:                  q.end,
 
                 eqFilts: q.eqFilts,
 
                 ineqFiltProp:     q.ineqFiltProp,
                 ineqFiltLow:      q.ineqFiltLow,
                 ineqFiltLowIncl:  q.ineqFiltLowIncl,
                 ineqFiltLowSet:   q.ineqFiltLowSet,
                 ineqFiltHigh:     q.ineqFiltHigh,
                 ineqFiltHighIncl: q.ineqFiltHighIncl,
                 ineqFiltHighSet:  q.ineqFiltHighSet,
         }
 
         if q.project != nil {
                 ret.project = q.project.ToSlice()
                 ret.distinct = q.distinct && q.project.Len() > 0
         }
 
+        seenOrders := stringset.New(len(q.order))
+
         // if len(q.order) > 0, we already enforce that the first order
         // is the same as the inequality above. Otherwise we need to add it.
         if len(q.order) == 0 && q.ineqFiltProp != "" {
                 ret.orders = []IndexColumn{{Property: q.ineqFiltProp}}
+                seenOrders.Add(q.ineqFiltProp)
         }
 
-        seenOrders := stringset.New(len(q.order))
-
         // drop orders where there's an equality filter
         //   https://cloud.google.com/appengine/docs/go/datastore/queries#sort_orders_are_ignored_on_properties_with_equality_filters
         // Deduplicate orders
         for _, o := range q.order {
                 if _, iseq := q.eqFilts[o.Property]; !iseq {
                         if seenOrders.Add(o.Property) {
                                 ret.orders = append(ret.orders, o)
                         }
                 }
         }
@@ skipping 130 matching lines @@
         if q.keysOnly {
                 p("KeysOnly")
         }
 
         if _, err := ret.WriteRune(')'); err != nil {
                 panic(err)
         }
 
         return ret.String()
 }