address comments

docs/design/README.md

@@ -22,7 +22,6 @@ Writing a design document can promote us to think deliberately and gather knowle
 
 - [Proposal: A new command to restore dropped table](./2018-08-10-restore-dropped-table.md)
 - [Proposal: Support SQL Plan Management](./2018-12-11-sql-plan-management.md)
-- [Proposal: Support Skyline Pruning](./2019-01-25-skyline-pruning.md)
 
 ### In Progress
 
@@ -39,3 +38,4 @@ Writing a design document can promote us to think deliberately and gather knowle
 
 - [Proposal: A new aggregate function execution framework](./2018-07-01-refactor-aggregate-framework.md)
 - [Proposal: Infer the System Timezone of a TiDB cluster via TZ environment variable](./2018-09-10-adding-tz-env.md)
+- [Proposal: Support Skyline Pruning](./2019-01-25-skyline-pruning.md)

expression/util.go

@@ -28,6 +28,7 @@ import (
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/types/parser_driver"
 	"github.com/pingcap/tidb/util/chunk"
+	"golang.org/x/tools/container/intsets"
 )
 
 // Filter the input expressions, append the results to result.
@@ -93,18 +94,18 @@ func extractColumns(result []*Column, expr Expression, filter func(*Column) bool
 }
 
 // ExtractColumnSet extract columns that occurred in the exprs.
-func ExtractColumnSet(exprs []Expression) map[int64]struct{} {
-	set := make(map[int64]struct{})
+func ExtractColumnSet(exprs []Expression) *intsets.Sparse {
+	set := &intsets.Sparse{}
 	for _, expr := range exprs {
 		extractColumnSet(expr, set)
 	}
 	return set
 }
 
-func extractColumnSet(expr Expression, set map[int64]struct{}) {
+func extractColumnSet(expr Expression, set *intsets.Sparse) {
 	switch v := expr.(type) {
 	case *Column:
-		set[v.UniqueID] = struct{}{}
+		set.Insert(int(v.UniqueID))
 	case *ScalarFunction:
 		for _, arg := range v.GetArgs() {
 			extractColumnSet(arg, set)

go.mod

@@ -79,7 +79,7 @@ require (
 	golang.org/x/sys v0.0.0-20190109145017-48ac38b7c8cb // indirect
 	golang.org/x/text v0.3.0
 	golang.org/x/time v0.0.0-20181108054448-85acf8d2951c // indirect
-	golang.org/x/tools v0.0.0-20190130214255-bb1329dc71a0 // indirect
+	golang.org/x/tools v0.0.0-20190130214255-bb1329dc71a0
 	google.golang.org/genproto v0.0.0-20190108161440-ae2f86662275 // indirect
 	google.golang.org/grpc v1.17.0
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0

planner/core/find_best_task.go

@@ -24,6 +24,7 @@ import (
 	"github.com/pingcap/tidb/planner/property"
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/util/chunk"
+	"golang.org/x/tools/container/intsets"
 )
 
 const (
@@ -199,29 +200,26 @@ func (ds *DataSource) tryToGetDualTask() (task, error) {
 // candidatePath is used to maintain required info for skyline pruning.
 type candidatePath struct {
 	path       *accessPath
-	columnSet  map[int64]struct{} // columnSet is the set of columns that occurred in the access conditions.
+	columnSet  *intsets.Sparse // columnSet is the set of columns that occurred in the access conditions.
 	singleScan bool
 	matchProp  bool
 }
 
 // compareColumnSet will compares the two set. The last return value is used to indicate
 // if they are comparable, it is false when both two sets have columns that do not occur in the other.
-func compareColumnSet(l, r map[int64]struct{}) (int, bool) {
-	if len(l) <= len(r) {
-		for key := range l {
-			if _, ok := r[key]; !ok {
-				return 0, false
-			}
+func compareColumnSet(l, r *intsets.Sparse) (int, bool) {
+	lLen, rLen := l.Len(), r.Len()
+	if lLen <= rLen {
+		if isSubset := l.SubsetOf(r); !isSubset {
+			return 0, false
 		}
-		if len(l) == len(r) {
+		if lLen == rLen {
 			return 0, true
 		}
 		return -1, true
 	}
-	for key := range r {
-		if _, ok := l[key]; !ok {
-			return 0, false
-		}
+	if isSubset := r.SubsetOf(l); !isSubset {
+		return 0, false
 	}
 	return 1, true
 }
@@ -239,7 +237,7 @@ func compareBool(l, r bool) int {
 // compareCandidates is the core of skyline pruning. It compares the two candidate paths on three dimensions:
 // the set of columns that occurred in the access condition, whether or not it matches the physical property
 // and does it require a double scan.  If `x` is not worse than `y` at all factors,
-// and there exists one factor that `x` is better than `y`, then we `x` is better than `y`.
+// and there exists one factor that `x` is better than `y`, then `x` is better than `y`.
 func compareCandidates(lhs, rhs *candidatePath) int {
 	setsResult, comparable := compareColumnSet(lhs.columnSet, rhs.columnSet)
 	if !comparable {