Refactor keys and queries in datastore service and implementation.

service/datastore/finalized_query.go

Index: service/datastore/finalized_query.go
diff --git a/service/datastore/finalized_query.go b/service/datastore/finalized_query.go
new file mode 100644
index 0000000000000000000000000000000000000000..4b6d854aff4610e8bea888e13dad5dd1599f1d88
--- /dev/null
+++ b/service/datastore/finalized_query.go
@@ -0,0 +1,257 @@
+// 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"
+	"strings"
+)
+
+type FinalizedQuery struct {
+	original *Query
+
+	kind                 string
+	eventuallyConsistent bool
+	distinct             bool
+	keysOnly             bool
+
+	limit  *int32
+	offset *int32
+
+	start Cursor
+	end   Cursor
+
+	project []string
+	orders  []IndexColumn
+
+	eqFilts map[string]PropertySlice
+
+	ineqFiltProp     string
+	ineqFiltLow      Property
+	ineqFiltLowIncl  bool
+	ineqFiltLowSet   bool
+	ineqFiltHigh     Property
+	ineqFiltHighIncl bool
+	ineqFiltHighSet  bool
+}
+
+func (q *FinalizedQuery) Original() *Query {

[iannucci, 2015/09/18 04:31:53]

yeah, I know I need docstrings on all of these.

[iannucci, 2015/09/18 04:31:53]

yeah, I know I need docstrings on all of these.

[dnj, 2015/09/18 16:47:57]

yeah, I know you know you need docstrings on all of these.

+	return q.original
+}
+
+func (q *FinalizedQuery) Kind() string {
+	return q.kind
+}
+
+func (q *FinalizedQuery) EventuallyConsistent() bool {
+	return q.eventuallyConsistent
+}
+
+func (q *FinalizedQuery) Project() []string {
+	ret := make([]string, len(q.project))

[dnj, 2015/09/18 16:47:57]

Maybe an allocation-avoiding shortcut here:
if len(q.project) == 0 {
  return nil
}

[iannucci, 2015/09/18 22:25:48]

done

[dnj, 2015/09/18 22:45:12]

I think you missed this.

+	copy(ret, q.project)
+	return ret
+}
+
+func (q *FinalizedQuery) Distinct() bool {
+	return q.distinct
+}
+
+func (q *FinalizedQuery) KeysOnly() bool {
+	return q.keysOnly
+}
+
+func (q *FinalizedQuery) Limit() (int32, bool) {
+	if q.limit != nil {
+		return *q.limit, true
+	}
+	return 0, false
+}
+
+func (q *FinalizedQuery) Offset() (int32, bool) {
+	if q.offset != nil {
+		return *q.offset, true
+	}
+	return 0, false
+}
+
+func (q *FinalizedQuery) Orders() []IndexColumn {
+	ret := make([]IndexColumn, len(q.orders))
+	copy(ret, q.orders)
+	return ret
+}
+
+func (q *FinalizedQuery) Bounds() (start, end Cursor) {
+	return q.start, q.end
+}
+
+func (q *FinalizedQuery) Ancestor() *Key {
+	if anc, ok := q.eqFilts["__ancestor__"]; ok {
+		return anc[0].Value().(*Key)
+	}
+	return nil
+}
+
+func (q *FinalizedQuery) EqFilters() map[string]PropertySlice {
+	ret := make(map[string]PropertySlice, len(q.eqFilts))
+	for k, v := range q.eqFilts {
+		newV := make(PropertySlice, len(v))
+		copy(newV, v)
+		ret[k] = newV
+	}
+	return ret
+}
+
+func (q *FinalizedQuery) IneqFilterProp() string {
+	return q.ineqFiltProp
+}
+
+func (q *FinalizedQuery) IneqFilterLow() (field, op string, val Property) {
+	if q.ineqFiltLowSet {
+		field = q.ineqFiltProp
+		val = q.ineqFiltLow
+		op = ">"

[dnj, 2015/09/18 16:47:57]

Since "op" is a return value, you can avoid the double-assignment with an
if/else.

[iannucci, 2015/09/18 22:25:48]

¯\_(ツ)_/¯

+		if q.ineqFiltLowIncl {
+			op = ">="
+		}
+	}
+	return
+}
+
+func (q *FinalizedQuery) IneqFilterHigh() (field, op string, val Property) {
+	if q.ineqFiltHighSet {
+		field = q.ineqFiltProp
+		val = q.ineqFiltHigh
+		op = "<"

[dnj, 2015/09/18 16:47:57]

Since "op" is a return value, you can avoid the double-assignment with an
if/else.

+		if q.ineqFiltHighIncl {
+			op = "<="
+		}
+	}
+	return
+}
+
+var escaper = strings.NewReplacer(
+	"\\%", `\%`,
+	"\\_", `\_`,
+	"\\", `\\`,
+	"\x00", `\0`,
+	"\b", `\b`,
+	"\n", `\n`,
+	"\r", `\r`,
+	"\t", `\t`,
+	"\x1A", `\Z`,
+	"'", `\'`,
+	"\"", `\"`,
+	"`", "\\`",
+)
+
+func gqlQuoteName(s string) string {
+	return fmt.Sprintf("`%s`", escaper.Replace(s))
+}
+
+func gqlQuoteString(s string) string {
+	return fmt.Sprintf(`"%s"`, escaper.Replace(s))
+}
+
+func (q *FinalizedQuery) GQL() string {

[dnj, 2015/09/18 16:47:57]

Make sure you mention which dialect of GQL you're using.

+	ret := bytes.Buffer{}
+
+	ret.WriteString("SELECT")
+	if len(q.project) != 0 {
+		if q.distinct {
+			ret.WriteString(" DISTINCT")
+		}
+		proj := make([]string, len(q.project))
+		for i, p := range q.project {
+			proj[i] = gqlQuoteName(p)
+		}
+		ret.WriteString(" ")
+		ret.WriteString(strings.Join(proj, ", "))

[dnj, 2015/09/18 16:47:57]

nit: Since we're already leaning heavily on the Buffer's resizing, I'm not sure
there's much value to separate alloc + Join instead of:
for i, p := range q.project {
  if i > 0 {
    ret.WriteString(", ")
  } else {
    ret.WriteString(" ")
  }
  ret.WriteString(gqlQuoteName(p))
}

[iannucci, 2015/09/18 22:25:48]

if there are a bunch of projections, that's a lot of potential re-allocations.
If buffer was internally using a `rope` structure, then yeah, but it's not 
(that'd be a good optimization though...)

+	} else {
+		ret.WriteString(" *")
+	}
+
+	if q.kind != "" {
+		fmt.Fprintf(&ret, " FROM %s", gqlQuoteName(q.kind))
+	}
+
+	filts := []string(nil)
+	if len(q.eqFilts) > 0 {
+		eqProps := make([]string, 0, len(q.eqFilts))
+		for k := range q.eqFilts {
+			if k == "__ancestor__" {

[dnj, 2015/09/18 16:47:57]

Retain this for later so you don't have to double-lookup. Also adds some
continuity.

[iannucci, 2015/09/18 22:25:48]

done

+				continue
+			}
+			eqProps = append(eqProps, k)
+		}
+		sort.Strings(eqProps)
+		for _, k := range eqProps {
+			vals := q.eqFilts[k]
+			k = gqlQuoteName(k)
+			for _, v := range vals {
+				if v.Type() == PTNull {
+					filts = append(filts, fmt.Sprintf("%s IS NULL", k))
+				} else {
+					filts = append(filts, fmt.Sprintf("%s = %s", k, v.GQL()))
+				}
+			}
+		}
+	}
+	if q.ineqFiltProp != "" {
+		for _, f := range [](func() (p, op string, v Property)){q.IneqFilterLow, q.IneqFilterHigh} {
+			prop, op, v := f()
+			if prop != "" {
+				filts = append(filts, fmt.Sprintf("%s %s %s", gqlQuoteName(prop), op, v.GQL()))
+			}
+		}
+	}
+	if anc, ok := q.eqFilts["__ancestor__"]; ok {

[dnj, 2015/09/18 16:47:57]

Use retained value here.

+		filts = append(filts, fmt.Sprintf("__key__ HAS ANCESTOR %s", anc[0].GQL()))
+	}
+	if len(filts) > 0 {
+		fmt.Fprintf(&ret, " WHERE %s", strings.Join(filts, " AND "))
+	}
+
+	if len(q.orders) > 0 {
+		orders := make([]string, len(q.orders))
+		for i, col := range q.orders {
+			orders[i] = col.GQL()
+		}
+		fmt.Fprintf(&ret, " ORDER BY %s", strings.Join(orders, ", "))
+	}
+
+	if q.limit != nil {
+		fmt.Fprintf(&ret, " LIMIT %d", *q.limit)
+	}
+	if q.offset != nil {
+		fmt.Fprintf(&ret, " OFFSET %d", *q.offset)
+	}
+
+	return ret.String()
+}
+
+func (q *FinalizedQuery) String() string {
+	// TODO(riannucci): make a more compact go-like representation here.
+	return q.GQL()
+}
+
+func (q *FinalizedQuery) Valid(aid, ns string) error {
+	anc := q.Ancestor()
+	if anc != nil && (!anc.Valid(false, aid, ns) || anc.Incomplete()) {
+		return ErrInvalidKey
+	}
+
+	if q.ineqFiltProp == "__key__" {
+		if q.ineqFiltLowSet && !q.ineqFiltLow.Value().(*Key).Valid(false, aid, ns) {
+			return ErrInvalidKey
+		}
+		if q.ineqFiltHighSet && !q.ineqFiltHigh.Value().(*Key).Valid(false, aid, ns) {
+			return ErrInvalidKey
+		}
+	}
+	return nil
+}