sql/{parse,plan}: Add support union parsing and execution. #68
Conversation
This is still partial. We need type coercion and schema validation to be done in the analysis phase.
| @@ -1507,6 +1507,26 @@ var fixtures = map[string]sql.Node{ | |||
| `SELECT * FROM foo`, | |||
| true, | |||
| ), | |||
| `SELECT 2 UNION SELECT 3` : plan.NewUnion( | |||
There was a problem hiding this comment.
Probably should add a couple tests for distinct, as well as unions of 3 or more select statements
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return ui.cur.Next() |
There was a problem hiding this comment.
Probably should recurse here instead
There was a problem hiding this comment.
I think it's (much) clearer without recursion. Am I crazy?
There was a problem hiding this comment.
The issue is that this is only correct when N=2. For the general case of composing an iterator from N sub iterators, if ui.cur.Next() on line 87 returns (nil, io.EOF), then recursion correctly continues to the next element, but what you have now terminates early.
There was a problem hiding this comment.
But that's not what this method does...agreed if this iterator took a bunch of row iterators and concat'd them, this impl would need to be different.
There was a problem hiding this comment.
I still think recursion is cleaner, but you do you :p
…the schema of both sides of a union is the same.
| return false | ||
| } | ||
| for i := range ls { | ||
| if !reflect.DeepEqual(ls[i].Type, rs[i].Type) { |
There was a problem hiding this comment.
Seems like this is mutually exclusive with the type conversion code in mergeUnionSchemas
There was a problem hiding this comment.
It is, but it's supposed to be a sanity check step in case schema merge fails.
| memory.NewTable( | ||
| "mytable", | ||
| sql.Schema{ | ||
| {Name: "foo", Source: "mytable"}, |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return ui.cur.Next() |
There was a problem hiding this comment.
The issue is that this is only correct when N=2. For the general case of composing an iterator from N sub iterators, if ui.cur.Next() on line 87 returns (nil, io.EOF), then recursion correctly continues to the next element, but what you have now terminates early.
This is still partial. We need type coercion and schema validation to be done
in the analysis phase.