Merge pull request #330 from upper/issue-310

Don't ignore errors from (internal) FindTablePrimaryKeys

.travis.yml

@@ -24,7 +24,14 @@ addons:
  - mongodb-org-server
  - mongodb-org-shell
 
-env: GOARCH=amd64 DB_HOST=127.0.0.1
+env:
+ global:
+ - MAKEFLAGS="-j4"
+ - GOARCH=amd64
+ - DB_HOST=127.0.0.1
+ matrix:
+ - TEST_CMD="make benchmark test-main"
+ - TEST_CMD="make test-adapters"
 
 install:
  - mkdir -p $GOPATH/src/upper.io
@@ -38,5 +45,4 @@ before_script:
  - mysql_tzinfo_to_sql /usr/share/zoneinfo | mysql -u root mysql
 
 script:
-# - UPPERIO_DB_DEBUG=1 make test
- - make test
+ - ${TEST_CMD}

Makefile

@@ -4,10 +4,35 @@ DB_HOST ?= 127.0.0.1
 
 export DB_HOST
 
-test:
- go test -v -benchtime=500ms -bench=. ./lib/... && \
- go test -v -benchtime=500ms -bench=. ./internal/... && \
- for ADAPTER in postgresql mysql sqlite ql mongo; do \
- $(MAKE) -C $$ADAPTER test || exit 1; \
- done && \
+benchmark-lib:
+ go test -v -benchtime=500ms -bench=. ./lib/...
+
+benchmark-internal:
+ go test -v -benchtime=500ms -bench=. ./internal/...
+
+benchmark: benchmark-lib benchmark-internal
+
+test-lib:
+ go test -v ./lib/...
+
+test-internal:
+ go test -v ./internal/...
+
+test-libs: test-lib test-internal
+
+test-adapters: test-adapter-postgresql test-adapter-mysql test-adapter-sqlite test-adapter-ql test-adapter-mongo
+
+reset-db:
+ $(MAKE) -C postgresql reset-db && \
+ $(MAKE) -C mysql reset-db && \
+ $(MAKE) -C sqlite reset-db && \
+ $(MAKE) -C ql reset-db && \
+ $(MAKE) -C mongo reset-db
+
+test-main: reset-db
  go test -v
+
+test: test-adapters test-libs test-main
+
+test-adapter-%:
+ $(MAKE) -C $* test || exit 1;

internal/sqladapter/collection.go

@@ -36,14 +36,15 @@ type condsFilter interface {
 
 // collection is the implementation of Collection.
 type collection struct {
- p PartialCollection
- pk []string
+ p PartialCollection
+ pk []string
+ err error
 }
 
 // NewBaseCollection returns a collection with basic methods.
 func NewBaseCollection(p PartialCollection) BaseCollection {
  c := &collection{p: p}
- c.pk, _ = c.p.Database().FindTablePrimaryKeys(c.p.Name())
+ c.pk, c.err = c.p.Database().FindTablePrimaryKeys(c.p.Name())
  return c
 }
 
@@ -66,11 +67,14 @@ func (c *collection) filterConds(conds ...interface{}) []interface{} {
 
 // Find creates a result set with the given conditions.
 func (c *collection) Find(conds ...interface{}) db.Result {
- return NewResult(
- c.p.Database(),
- c.p.Name(),
- c.filterConds(conds...),
- )
+ if c.err != nil {
+ return &Result{err: c.err}
+ }
+ return &Result{
+ b: c.p.Database(),
+ table: c.p.Name(),
+ conds: [][]interface{}{c.filterConds(conds...)},
+ }
 }
 
 // Exists returns true if the collection exists.

internal/sqladapter/result.go

@@ -45,16 +45,6 @@ type Result struct {
  iterMu sync.Mutex
 }
 
-// NewResult creates and Results a new Result set on the given table, this set
-// is limited by the given exql.Where conditions.
-func NewResult(b sqlbuilder.Builder, table string, conds []interface{}) *Result {
- return &Result{
- b: b,
- table: table,
- conds: [][]interface{}{conds},
- }
-}
-
 func (r *Result) setErr(err error) error {
  if err == nil {
  return nil
@@ -128,12 +118,18 @@ func (r *Result) String() string {
 
 // All dumps all Results into a pointer to an slice of structs or maps.
 func (r *Result) All(dst interface{}) error {
+ if err := r.Err(); err != nil {
+ return err
+ }
  err := r.buildSelect().Iterator().All(dst)
  return r.setErr(err)
 }
 
 // One fetches only one Result from the set.
 func (r *Result) One(dst interface{}) error {
+ if err := r.Err(); err != nil {
+ return err
+ }
  err := r.buildSelect().Iterator().One(dst)
  return r.setErr(err)
 }
@@ -157,6 +153,10 @@ func (r *Result) Next(dst interface{}) bool {
 
 // Delete deletes all matching items from the collection.
 func (r *Result) Delete() error {
+ if err := r.Err(); err != nil {
+ return err
+ }
+
  q := r.b.DeleteFrom(r.table).
  Limit(r.limit)
 
@@ -170,6 +170,9 @@ func (r *Result) Delete() error {
 
 // Close closes the Result set.
 func (r *Result) Close() error {
+ if err := r.Err(); err != nil {
+ return err
+ }
  if r.iter != nil {
  return r.setErr(r.iter.Close())
  }
@@ -179,6 +182,9 @@ func (r *Result) Close() error {
 // Update updates matching items from the collection with values of the given
 // map or struct.
 func (r *Result) Update(values interface{}) error {
+ if err := r.Err(); err != nil {
+ return err
+ }
  q := r.b.Update(r.table).
  Set(values).
  Limit(r.limit)
@@ -193,6 +199,10 @@ func (r *Result) Update(values interface{}) error {
 
 // Count counts the elements on the set.
 func (r *Result) Count() (uint64, error) {
+ if err := r.Err(); err != nil {
+ return 0, err
+ }
+
  counter := struct {
  Count uint64 `db:"_t"`
  }{}

postgresql/database.go

@@ -23,6 +23,7 @@ package postgresql
 
 import (
  "database/sql"
+ "fmt"
  "strings"
  "sync"
 
@@ -237,12 +238,22 @@ func (d *database) TableExists(name string) error {
  return db.ErrCollectionDoesNotExist
 }
 
+// quotedTableName returns a valid regclass name for both regular tables and
+// for schemas.
+func quotedTableName(s string) string {
+ chunks := strings.Split(s, ".")
+ for i := range chunks {
+ chunks[i] = fmt.Sprintf("%q", chunks[i])
+ }
+ return strings.Join(chunks, ".")
+}
+
 // FindTablePrimaryKeys allows sqladapter find a table's primary keys.
 func (d *database) FindTablePrimaryKeys(tableName string) ([]string, error) {
  q := d.Select("pg_attribute.attname AS pkey").
  From("pg_index", "pg_class", "pg_attribute").
  Where(`
- pg_class.oid = '` + tableName + `'::regclass
+ pg_class.oid = '` + quotedTableName(tableName) + `'::regclass
  AND indrelid = pg_class.oid
  AND pg_attribute.attrelid = pg_class.oid
  AND pg_attribute.attnum = ANY(pg_index.indkey)
@@ -261,6 +272,8 @@ func (d *database) FindTablePrimaryKeys(tableName string) ([]string, error) {
  }
  pk = append(pk, k)
  }
-
+ if err := iter.Err(); err != nil {
+ return nil, err
+ }
  return pk, nil
 }