fix #127. Invalid delete queries should return an error
Don't allow where conditions other than 'time > and time <' in delete queriespull/144/head
parent
8b3b1d42ca
commit
fe53533528
|
@ -788,6 +788,36 @@ func (self *IntegrationSuite) TestDbDelete(c *C) {
|
|||
c.Assert(data[0].Points, HasLen, 0)
|
||||
}
|
||||
|
||||
func (self *IntegrationSuite) TestInvalidDeleteQuery(c *C) {
|
||||
err := self.server.WriteData(`
|
||||
[
|
||||
{
|
||||
"name": "test_invalid_delete_query",
|
||||
"columns": ["val1", "val2"],
|
||||
"points":[["v1", 2]]
|
||||
}
|
||||
]`)
|
||||
c.Assert(err, IsNil)
|
||||
bs, err := self.server.RunQuery("select val1 from test_invalid_delete_query")
|
||||
c.Assert(err, IsNil)
|
||||
data := []*h.SerializedSeries{}
|
||||
err = json.Unmarshal(bs, &data)
|
||||
c.Assert(data, HasLen, 1)
|
||||
c.Assert(data[0].Points, HasLen, 1)
|
||||
|
||||
_, err = self.server.RunQuery("delete from test_invalid_delete_query where foo = 'bar'")
|
||||
c.Assert(err, ErrorMatches, ".*don't reference time.*")
|
||||
|
||||
// this shouldn't return any data
|
||||
bs, err = self.server.RunQuery("select val1 from test_invalid_delete_query")
|
||||
c.Assert(err, IsNil)
|
||||
data = []*h.SerializedSeries{}
|
||||
err = json.Unmarshal(bs, &data)
|
||||
c.Assert(err, IsNil)
|
||||
c.Assert(data, HasLen, 1)
|
||||
c.Assert(data[0].Points, HasLen, 1)
|
||||
}
|
||||
|
||||
// test delete query
|
||||
func (self *IntegrationSuite) TestDeleteQuery(c *C) {
|
||||
for _, queryString := range []string{
|
||||
|
|
|
@ -493,5 +493,8 @@ func parseDeleteQuery(queryString string, query *C.delete_query) (*DeleteQuery,
|
|||
goQuery := &DeleteQuery{
|
||||
SelectDeleteCommonQuery: basicQurey,
|
||||
}
|
||||
if basicQurey.GetWhereCondition() != nil {
|
||||
return nil, fmt.Errorf("Delete queries can't have where clause that don't reference time")
|
||||
}
|
||||
return goQuery, nil
|
||||
}
|
||||
|
|
|
@ -218,13 +218,22 @@ func (self *QueryParserSuite) TestParseSelectWithInequality(c *C) {
|
|||
}
|
||||
|
||||
func (self *QueryParserSuite) TestParseSelectWithTimeCondition(c *C) {
|
||||
q, err := ParseSelectQuery("select value, time from t where time > now() - 1d;")
|
||||
c.Assert(err, IsNil)
|
||||
queries := map[string]time.Time{
|
||||
"select value, time from t where time > now() - 1d and time < now() - 1m;": time.Now().Add(-time.Minute).Round(time.Minute).UTC(),
|
||||
"select value, time from t where time > now() - 1d and time < now();": time.Now().Round(time.Minute).UTC(),
|
||||
}
|
||||
for query, expected := range queries {
|
||||
fmt.Printf("Running %s\n", query)
|
||||
|
||||
// note: the time condition will be removed
|
||||
c.Assert(q.GetWhereCondition(), IsNil)
|
||||
q, err := ParseSelectQuery(query)
|
||||
c.Assert(err, IsNil)
|
||||
|
||||
c.Assert(q.GetStartTime().Round(time.Minute), Equals, time.Now().Add(-24*time.Hour).Round(time.Minute).UTC())
|
||||
c.Assert(q.GetStartTime().Round(time.Minute), Equals, time.Now().Add(-24*time.Hour).Round(time.Minute).UTC())
|
||||
c.Assert(q.GetEndTime().Round(time.Minute).UTC(), Equals, expected)
|
||||
|
||||
// note: the time condition will be removed
|
||||
c.Assert(q.GetWhereCondition(), IsNil)
|
||||
}
|
||||
}
|
||||
|
||||
func (self *QueryParserSuite) TestParseSelectWithPartialTimeString(c *C) {
|
||||
|
|
|
@ -325,35 +325,35 @@ func getTime(condition *WhereCondition, isParsingStartTime bool) (*WhereConditio
|
|||
|
||||
if expr, ok := condition.GetBoolExpression(); ok {
|
||||
leftValue := expr.Elems[0]
|
||||
isLeftValue := leftValue.Type != ValueExpression
|
||||
isTimeOnLeft := leftValue.Type != ValueExpression && leftValue.Type != ValueFunctionCall
|
||||
rightValue := expr.Elems[1]
|
||||
isRightValue := rightValue.Type != ValueExpression
|
||||
isTimeOnRight := rightValue.Type != ValueExpression && rightValue.Type != ValueFunctionCall
|
||||
|
||||
// this can only be the case if the where condition
|
||||
// is of the form `"time" > 123456789`, so let's see
|
||||
// which side is a float value
|
||||
if isLeftValue && isRightValue {
|
||||
if isTimeOnLeft && isTimeOnRight {
|
||||
if isNumericValue(rightValue) {
|
||||
isRightValue = false
|
||||
isTimeOnRight = false
|
||||
} else {
|
||||
isLeftValue = false
|
||||
isTimeOnLeft = false
|
||||
}
|
||||
}
|
||||
|
||||
// if this expression isn't "time > xxx" or "xxx < time" then return
|
||||
// TODO: we should do a check to make sure "time" doesn't show up in
|
||||
// either expressions
|
||||
if !isLeftValue && !isRightValue {
|
||||
if !isTimeOnLeft && !isTimeOnRight {
|
||||
return condition, ZERO_TIME, nil
|
||||
}
|
||||
|
||||
var timeExpression *Value
|
||||
if !isRightValue {
|
||||
if !isTimeOnRight {
|
||||
if leftValue.Name != "time" {
|
||||
return condition, ZERO_TIME, nil
|
||||
}
|
||||
timeExpression = rightValue
|
||||
} else if !isLeftValue {
|
||||
} else if !isTimeOnLeft {
|
||||
if rightValue.Name != "time" {
|
||||
return condition, ZERO_TIME, nil
|
||||
}
|
||||
|
@ -364,11 +364,11 @@ func getTime(condition *WhereCondition, isParsingStartTime bool) (*WhereConditio
|
|||
|
||||
switch expr.Name {
|
||||
case ">":
|
||||
if isParsingStartTime && !isLeftValue || !isParsingStartTime && !isRightValue {
|
||||
if isParsingStartTime && !isTimeOnLeft || !isParsingStartTime && !isTimeOnRight {
|
||||
return condition, ZERO_TIME, nil
|
||||
}
|
||||
case "<":
|
||||
if !isParsingStartTime && !isLeftValue || isParsingStartTime && !isRightValue {
|
||||
if !isParsingStartTime && !isTimeOnLeft || isParsingStartTime && !isTimeOnRight {
|
||||
return condition, ZERO_TIME, nil
|
||||
}
|
||||
default:
|
||||
|
|
Loading…
Reference in New Issue