Implement comprehensive query wildcard re-write #1632
Conversation
otoolep
force-pushed
the
wildcard_group_by
branch
2 times, most recently
from
0a7a151
to
28f0f5a
Compare
|
This changes uses dedicated code for |
|
Addresses issue #1624. |
|
@dgnorton -- could do with your review too. |
| @@ -1977,6 +1977,12 @@ func (s *Server) ExecuteQuery(q *influxql.Query, database string, user *User) Re | |||
|
|
|||
| // executeSelectStatement plans and executes a select statement against a database. | |||
| func (s *Server) executeSelectStatement(stmt *influxql.SelectStatement, database string, user *User) *Result { | |||
| // Perform any necessary query re-writing. | |||
| stmt, err := s.rewriteSelectStatement(stmt) | |||
There was a problem hiding this comment.
We should only rewrite if the statement actually has a wildcard. The statement struct should have a method that indicates if it has a wildcard.
|
+1 |
|
Are we going to handle wildcard expansion for multiple source measurements? e.g. |
|
Wildcard method added @pauldix @benbjohnson -- there are a couple of different places the check could be placed, of course. |
| @@ -1508,6 +1508,8 @@ func (p *Parser) parseUnaryExpr() (Expr, error) { | |||
| case DURATION_VAL: | |||
| v, _ := ParseDuration(lit) | |||
| return &DurationLiteral{Val: v}, nil | |||
| case MUL: | |||
| return &Wildcard{}, nil | |||
There was a problem hiding this comment.
Wondering if wildcard expressions should be parsed separately so we don't allow weird stuff like SELECT value + *,* FROM cpu, which will get expanded to look something like SELECT value + *, value1, value2 FROM cpu. /cc @benbjohnson
There was a problem hiding this comment.
Sounds like a reasonable suggestion, but outside the scope of this change I'd say. Happy to open an issue.
There was a problem hiding this comment.
I agree that we should do a lookahead for wildcards before parsing expressions but we can push that to a separate PR.
|
@pauldix is refactoring a chunk of the query engine, so this PR is on hold. He and @benbjohnson have promised to merge (or possibly rework) this when ready. |
This code, when given a SELECT statement, returns another SELECT statement such that all "query" and GROUP BY wildcards have been replaced with all Measurement fields and tag keys respectively.
GROUP BY * functionality is now in place.
otoolep
force-pushed
the
wildcard_group_by
branch
from
7b916cd
to
4d896fa
Compare
|
Rebased on master, and green build now. |
Implement comprehensive query wildcard re-write
This patch implements comprehensive rewrite of wildcards, for both the "source" of the
SELECTstatement, and theGROUP BY. The parser prevents statements such asSELECT *,value, so those unit tests are commented out. However, the query parser should be fixed up eventually to allow this, as other SQL-based query engines support this.