sql/parse: generate sort and limit before project. (#44)

* Sort was being generated after Project, so it was not possible to ORDER BY by a field not included in SELECT. * Limit was being executed after the Project too.
src-d · Nov 4, 2016 · 4e15e80 · 4e15e80
1 parent 01ace9e
commit 4e15e80
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 40 deletions.
diff --git a/engine_test.go b/engine_test.go
@@ -37,6 +37,20 @@ func TestEngine_Query(t *testing.T) {
 			sql.NewMemoryRow(int64(1)),
 		},
 	)
+
+	testQuery(t, e,
+		"SELECT i FROM mytable WHERE s = 'a' ORDER BY i DESC;",
+		[]sql.Row{
+			sql.NewMemoryRow(int64(1)),
+		},
+	)
+
+	testQuery(t, e,
+		"SELECT i FROM mytable WHERE s = 'a' ORDER BY i DESC LIMIT 1;",
+		[]sql.Row{
+			sql.NewMemoryRow(int64(1)),
+		},
+	)
 }
 
 func testQuery(t *testing.T, e *gitql.Engine, q string, r []sql.Row) {
@@ -66,10 +80,13 @@ func testQuery(t *testing.T, e *gitql.Engine, q string, r []sql.Row) {
 func newEngine(t *testing.T) *gitql.Engine {
 	assert := require.New(t)
 
-	table := mem.NewTable("mytable", sql.Schema{{"i", sql.BigInteger}})
-	assert.Nil(table.Insert(int64(1)))
-	assert.Nil(table.Insert(int64(2)))
-	assert.Nil(table.Insert(int64(3)))
+	table := mem.NewTable("mytable", sql.Schema{
+		{"i", sql.BigInteger},
+		{"s", sql.String},
+	})
+	assert.Nil(table.Insert(int64(1), "a"))
+	assert.Nil(table.Insert(int64(2), "b"))
+	assert.Nil(table.Insert(int64(3), "c"))
 
 	db := mem.NewDatabase("mydb")
 	db.AddTable("mytable", table)

diff --git a/sql/parse/parse.go b/sql/parse/parse.go
@@ -282,7 +282,6 @@ func (p *parser) buildPlan() (sql.Node, error) {
 		node = plan.NewFilter(p.filterClauses[0], node)
 	}
 
-	node = plan.NewProject(p.projection, node)
 	if len(p.sortFields) > 0 {
 		node = plan.NewSort(p.sortFields, node)
 	}
@@ -291,6 +290,8 @@ func (p *parser) buildPlan() (sql.Node, error) {
 		node = plan.NewLimit(int64(*p.limit), node)
 	}
 
+	node = plan.NewProject(p.projection, node)
+
 	return node, nil
 }
 

diff --git a/sql/parse/parse_test.go b/sql/parse/parse_test.go
@@ -45,31 +45,31 @@ var fixtures = map[string]sql.Node{
 			plan.NewUnresolvedRelation("foo"),
 		),
 	),
-	`SELECT foo, bar FROM foo LIMIT 10;`: plan.NewLimit(int64(10),
-		plan.NewProject(
-			[]sql.Expression{
-				expression.NewUnresolvedColumn("foo"),
-				expression.NewUnresolvedColumn("bar"),
-			},
+	`SELECT foo, bar FROM foo LIMIT 10;`: plan.NewProject(
+		[]sql.Expression{
+			expression.NewUnresolvedColumn("foo"),
+			expression.NewUnresolvedColumn("bar"),
+		},
+		plan.NewLimit(int64(10),
 			plan.NewUnresolvedRelation("foo"),
 		),
 	),
-	`SELECT foo, bar FROM foo ORDER BY baz DESC;`: plan.NewSort(
-		[]plan.SortField{{expression.NewUnresolvedColumn("baz"), plan.Descending}},
-		plan.NewProject(
-			[]sql.Expression{
-				expression.NewUnresolvedColumn("foo"),
-				expression.NewUnresolvedColumn("bar"),
-			},
+	`SELECT foo, bar FROM foo ORDER BY baz DESC;`: plan.NewProject(
+		[]sql.Expression{
+			expression.NewUnresolvedColumn("foo"),
+			expression.NewUnresolvedColumn("bar"),
+		},
+		plan.NewSort(
+			[]plan.SortField{{expression.NewUnresolvedColumn("baz"), plan.Descending}},
 			plan.NewUnresolvedRelation("foo"),
 		),
 	),
-	`SELECT foo, bar FROM foo WHERE foo = bar LIMIT 10;`: plan.NewLimit(int64(10),
-		plan.NewProject(
-			[]sql.Expression{
-				expression.NewUnresolvedColumn("foo"),
-				expression.NewUnresolvedColumn("bar"),
-			},
+	`SELECT foo, bar FROM foo WHERE foo = bar LIMIT 10;`: plan.NewProject(
+		[]sql.Expression{
+			expression.NewUnresolvedColumn("foo"),
+			expression.NewUnresolvedColumn("bar"),
+		},
+		plan.NewLimit(int64(10),
 			plan.NewFilter(
 				expression.NewEquals(
 					expression.NewUnresolvedColumn("foo"),
@@ -79,26 +79,26 @@ var fixtures = map[string]sql.Node{
 			),
 		),
 	),
-	`SELECT foo, bar FROM foo ORDER BY baz DESC LIMIT 1;`: plan.NewLimit(int64(1),
-		plan.NewSort(
-			[]plan.SortField{{expression.NewUnresolvedColumn("baz"), plan.Descending}},
-			plan.NewProject(
-				[]sql.Expression{
-					expression.NewUnresolvedColumn("foo"),
-					expression.NewUnresolvedColumn("bar"),
-				},
+	`SELECT foo, bar FROM foo ORDER BY baz DESC LIMIT 1;`: plan.NewProject(
+		[]sql.Expression{
+			expression.NewUnresolvedColumn("foo"),
+			expression.NewUnresolvedColumn("bar"),
+		},
+		plan.NewLimit(int64(1),
+			plan.NewSort(
+				[]plan.SortField{{expression.NewUnresolvedColumn("baz"), plan.Descending}},
 				plan.NewUnresolvedRelation("foo"),
 			),
 		),
 	),
-	`SELECT foo, bar FROM foo WHERE qux = 1 ORDER BY baz DESC LIMIT 1;`: plan.NewLimit(int64(1),
-		plan.NewSort(
-			[]plan.SortField{{expression.NewUnresolvedColumn("baz"), plan.Descending}},
-			plan.NewProject(
-				[]sql.Expression{
-					expression.NewUnresolvedColumn("foo"),
-					expression.NewUnresolvedColumn("bar"),
-				},
+	`SELECT foo, bar FROM foo WHERE qux = 1 ORDER BY baz DESC LIMIT 1;`: plan.NewProject(
+		[]sql.Expression{
+			expression.NewUnresolvedColumn("foo"),
+			expression.NewUnresolvedColumn("bar"),
+		},
+		plan.NewLimit(int64(1),
+			plan.NewSort(
+				[]plan.SortField{{expression.NewUnresolvedColumn("baz"), plan.Descending}},
 				plan.NewFilter(
 					expression.NewEquals(
 						expression.NewUnresolvedColumn("qux"),