Merge pull request #8754 from influxdata/js-8699-force-subqueries-to-match-ordering

Force subqueries to match the parent queries ordering

CHANGELOG.md

@@ -40,6 +40,7 @@
 - [#8712](https://github.com/influxdata/influxdb/pull/8712): Force time expressions to use AND and improve condition parsing.
 - [#8716](https://github.com/influxdata/influxdb/pull/8716): Ensure inputs are closed on error. Add runtime GC finalizer as additional guard to close iterators
 - [#8695](https://github.com/influxdata/influxdb/issues/8695): Fix merging bug on system iterators.
+- [#8699](https://github.com/influxdata/influxdb/issues/8699): Force subqueries to match the parent queries ordering.
 
 ## v1.3.4 [unreleased]
 

query/compile.go

@@ -38,6 +38,9 @@ type compiledStatement struct {
 	// a query that shouldn't have an interval to fail.
 	InheritedInterval bool
 
+	// Ascending is true if the time ordering is ascending.
+	Ascending bool
+
 	// FunctionCalls holds a reference to the call expression of every function
 	// call that has been encountered.
 	FunctionCalls []*influxql.Call
@@ -114,6 +117,7 @@ func Compile(stmt *influxql.SelectStatement, opt CompileOptions) (Statement, err
 
 // preprocess retrieves and records the global attributes of the current statement.
 func (c *compiledStatement) preprocess(stmt *influxql.SelectStatement) error {
+	c.Ascending = stmt.TimeAscending()
 	c.Limit = stmt.Limit
 	c.HasTarget = stmt.Target != nil
 
@@ -764,6 +768,13 @@ func (c *compiledStatement) subquery(stmt *influxql.SelectStatement) error {
 		return err
 	}
 
+	// If the ordering is different and the sort field was specified for the subquery,
+	// throw an error.
+	if len(stmt.SortFields) != 0 && subquery.Ascending != c.Ascending {
+		return errors.New("subqueries must be ordered in the same direction as the query itself")
+	}
+	subquery.Ascending = c.Ascending
+
 	// Find the intersection between this time range and the parent.
 	// If the subquery doesn't have a time range, this causes it to
 	// inherit the parent's time range.
@@ -804,6 +815,7 @@ func (c *compiledStatement) Prepare(shardMapper ShardMapper, sopt SelectOptions)
 		return nil, err
 	}
 	opt.StartTime, opt.EndTime = c.TimeRange.MinTime(), c.TimeRange.MaxTime()
+	opt.Ascending = c.Ascending
 
 	if sopt.MaxBucketsN > 0 && !stmt.IsRawQuery {
 		interval, err := stmt.GroupByInterval()

query/compile_test.go

@@ -79,6 +79,7 @@ func TestCompile_Success(t *testing.T) {
 		`SELECT max(derivative) FROM (SELECT derivative(mean(value)) FROM cpu) WHERE time >= now() - 1m GROUP BY time(10s)`,
 		`SELECT max(value) FROM (SELECT value + total FROM cpu) WHERE time >= now() - 1m GROUP BY time(10s)`,
 		`SELECT value FROM cpu WHERE time >= '2000-01-01T00:00:00Z' AND time <= '2000-01-01T01:00:00Z'`,
+		`SELECT value FROM (SELECT value FROM cpu) ORDER BY time DESC`,
 	} {
 		t.Run(tt, func(t *testing.T) {
 			stmt, err := influxql.ParseStatement(tt)
@@ -319,6 +320,7 @@ func TestCompile_Failures(t *testing.T) {
 		{s: `SELECT min(mean) FROM (SELECT mean(value) FROM myseries GROUP BY time)`, err: `time() is a function and expects at least one argument`},
 		{s: `SELECT value FROM myseries WHERE value OR time >= now() - 1m`, err: `invalid condition expression: value`},
 		{s: `SELECT value FROM myseries WHERE time >= now() - 1m OR value`, err: `invalid condition expression: value`},
+		{s: `SELECT value FROM (SELECT value FROM cpu ORDER BY time DESC) ORDER BY time ASC`, err: `subqueries must be ordered in the same direction as the query itself`},
 	} {
 		t.Run(tt.s, func(t *testing.T) {
 			stmt, err := influxql.ParseStatement(tt.s)

query/iterator.go

@@ -776,6 +776,9 @@ func newIteratorOptionsSubstatement(stmt *influxql.SelectStatement, opt Iterator
 	subOpt.SLimit += opt.SLimit
 	subOpt.SOffset += opt.SOffset
 
+	// Propagate the ordering from the parent query.
+	subOpt.Ascending = opt.Ascending
+
 	// If the inner query uses a null fill option, switch it to none so we
 	// don't hit an unnecessary penalty from the fill iterator. Null values
 	// will end up getting stripped by an outer query anyway so there's no