Merge pull request #91 from augmentable-dev/github-vtab-edits

modification to how github table filter and best index works
mergestat · Dec 27, 2020 · bed1424 · bed1424
2 parents 0f163dc + a44523e
commit bed1424
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 42 deletions.
diff --git a/pkg/ghqlite/pull_requests_vtab.go b/pkg/ghqlite/pull_requests_vtab.go
@@ -2,7 +2,6 @@ package ghqlite
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"math"
 	"strings"
@@ -38,7 +37,7 @@ func (m *PullRequestsModule) Create(c *sqlite3.SQLiteConn, args []string) (sqlit
 			repo_owner HIDDEN,
 			repo_name HIDDEN,
 			id INT,
-			node_id TEXT,
+			node_id TEXT PRIMARY KEY,
 			number INT,
 			state TEXT,
 			locked BOOL,
@@ -76,7 +75,7 @@ func (m *PullRequestsModule) Create(c *sqlite3.SQLiteConn, args []string) (sqlit
 			additions INT,
 			deletions INT,
 			changed_files INT
-		)`, args[0]))
+		) WITHOUT ROWID`, args[0]))
 	if err != nil {
 		return nil, err
 	}
@@ -236,42 +235,31 @@ func (v *pullRequestsTable) BestIndex(constraints []sqlite3.InfoConstraint, ob [
 	used := make([]bool, len(constraints))
 	var repoOwnerCstUsed, repoNameCstUsed bool
 	idxNameVals := make([]string, 0)
+	cost := 1000.0
 	for c, cst := range constraints {
+		if !cst.Usable {
+			continue
+		}
+		if cst.Op != sqlite3.OpEQ {
+			continue
+		}
 		switch cst.Column {
 		case 0: // repo_owner
-			if cst.Op != sqlite3.OpEQ { // if there's no equality constraint, fail
-				return nil, sqlite3.ErrConstraint
-			}
-			// if the constraint is usable, use it, otherwise fail
-			if used[c] = cst.Usable; !used[c] {
-				return nil, sqlite3.ErrConstraint
-			}
+			used[c] = true
 			repoOwnerCstUsed = true
 			idxNameVals = append(idxNameVals, "repo_owner")
 		case 1: // repo_name
-			if cst.Op != sqlite3.OpEQ { // if there's no equality constraint, fail
-				return nil, sqlite3.ErrConstraint
-			}
-			// if the constraint is usable, use it, otherwise fail
-			if used[c] = cst.Usable; !used[c] {
-				return nil, sqlite3.ErrConstraint
-			}
+			used[c] = true
 			repoNameCstUsed = true
 			idxNameVals = append(idxNameVals, "repo_name")
 		case 5:
-			if cst.Usable && cst.Op == sqlite3.OpEQ {
-				used[c] = true
-			}
+			used[c] = true
 			idxNameVals = append(idxNameVals, "state")
 		}
 	}
 
-	if !repoOwnerCstUsed {
-		return nil, errors.New("must supply a repo owner")
-	}
-
-	if !repoNameCstUsed {
-		return nil, errors.New("must supply a repo name")
+	if repoOwnerCstUsed && repoNameCstUsed {
+		cost = 0
 	}
 
 	var idxNum int
@@ -286,18 +274,19 @@ func (v *pullRequestsTable) BestIndex(constraints []sqlite3.InfoConstraint, ob [
 				idxNum = ob[0].Column
 			}
 		}
-
 	}
 
 	return &sqlite3.IndexResult{
 		IdxNum:         idxNum,
 		IdxStr:         strings.Join(idxNameVals, ","),
 		Used:           used,
 		AlreadyOrdered: alreadyOrdered,
+		EstimatedCost:  cost,
 	}, nil
 }
 
 func (vc *pullRequestsCursor) Filter(idxNum int, idxStr string, vals []interface{}) error {
+	vc.eof = false
 	state := "all"
 	for c, cstVal := range strings.Split(idxStr, ",") {
 		switch cstVal {

diff --git a/pkg/ghqlite/repos_vtab.go b/pkg/ghqlite/repos_vtab.go
@@ -2,7 +2,6 @@ package ghqlite
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"time"
 
@@ -37,7 +36,7 @@ func (m *ReposModule) Create(c *sqlite3.SQLiteConn, args []string) (sqlite3.VTab
 		CREATE TABLE %s (
 			repo_owner HIDDEN,
 			id INT,
-			node_id TEXT,
+			node_id TEXT PRIMARY KEY,
 			name TEXT,
 			full_name TEXT,
 			owner TEXT,
@@ -63,7 +62,7 @@ func (m *ReposModule) Create(c *sqlite3.SQLiteConn, args []string) (sqlite3.VTab
 			created_at DATETIME,
 			updated_at DATETIME,
 			permissions TEXT
-		)`, args[0]))
+		) WITHOUT ROWID`, args[0]))
 	if err != nil {
 		return nil, err
 	}
@@ -167,33 +166,36 @@ func (vc *reposCursor) Column(c *sqlite3.SQLiteContext, col int) error {
 
 func (v *reposTable) BestIndex(constraints []sqlite3.InfoConstraint, ob []sqlite3.InfoOrderBy) (*sqlite3.IndexResult, error) {
 	used := make([]bool, len(constraints))
+	cost := 1000.0
 	repoOwnerCstUsed := false
 	for c, cst := range constraints {
+		if !cst.Usable {
+			continue
+		}
+		if cst.Op != sqlite3.OpEQ {
+			continue
+		}
 		switch cst.Column {
 		case 0: // repo_owner
-			if cst.Op != sqlite3.OpEQ { // if there's no equality constraint, fail
-				return nil, sqlite3.ErrConstraint
-			}
-			// if the constraint is usable, use it, otherwise fail
-			if used[c] = cst.Usable; !used[c] {
-				return nil, sqlite3.ErrConstraint
-			}
+			used[c] = true
 			repoOwnerCstUsed = true
 		}
 	}
 
-	if !repoOwnerCstUsed {
-		return nil, errors.New("must supply a repo owner")
+	if repoOwnerCstUsed {
+		cost = 0
 	}
 
 	return &sqlite3.IndexResult{
-		IdxNum: 0,
-		IdxStr: "default",
-		Used:   used,
+		IdxNum:        0,
+		IdxStr:        "default",
+		Used:          used,
+		EstimatedCost: cost,
 	}, nil
 }
 
 func (vc *reposCursor) Filter(idxNum int, idxStr string, vals []interface{}) error {
+	vc.eof = false
 	owner := vals[0].(string)
 	iter := NewRepoIterator(owner, vc.table.module.ownerType, GitHubIteratorOptions{
 		Token:        vc.table.module.options.Token,