Skip to content

Commit 6634845

Browse files
[fix](parser) Validate ORDER BY must come before LIMIT
Add validation to ensure ORDER BY appears before LIMIT in SQL queries. - Add two-layer detection in withQueryOrganization method - Use LogicalSubQueryAlias as boundary marker - Add 17 comprehensive test cases
1 parent 4eba7f9 commit 6634845

2 files changed

Lines changed: 119 additions & 0 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4369,6 +4369,37 @@ private LogicalPlan withQueryOrganization(LogicalPlan inputPlan, QueryOrganizati
43694369
}
43704370
Optional<SortClauseContext> sortClauseContext = Optional.ofNullable(ctx.sortClause());
43714371
Optional<LimitClauseContext> limitClauseContext = Optional.ofNullable(ctx.limitClause());
4372+
4373+
// Check if LIMIT appears before ORDER BY (invalid SQL syntax)
4374+
if (sortClauseContext.isPresent() && limitClauseContext.isPresent()) {
4375+
int sortStart = sortClauseContext.get().getStart().getStartIndex();
4376+
int limitStart = limitClauseContext.get().getStart().getStartIndex();
4377+
if (limitStart < sortStart) {
4378+
throw new ParseException(
4379+
"Syntax error: ORDER BY must come before LIMIT. "
4380+
+ "Please rewrite your query as: ORDER BY ... LIMIT ...",
4381+
ctx);
4382+
}
4383+
}
4384+
4385+
// Check for "split across two calls" scenario: ORDER BY appearing after LIMIT
4386+
// This catches cases like "SELECT * FROM t LIMIT 10 ORDER BY a"
4387+
// where LIMIT is processed in first call and ORDER BY in second call
4388+
if (sortClauseContext.isPresent() && inputPlan instanceof LogicalLimit) {
4389+
LogicalLimit<?> limitPlan = (LogicalLimit<?>) inputPlan;
4390+
Plan child = limitPlan.child();
4391+
4392+
// Only reject if LIMIT and ORDER BY are in the same query level
4393+
// If child is LogicalSubQueryAlias, the LIMIT comes from a subquery (different level)
4394+
// e.g., "SELECT * FROM (SELECT * FROM t LIMIT 10) subq ORDER BY a" is valid
4395+
if (!(child instanceof LogicalSubQueryAlias)) {
4396+
throw new ParseException(
4397+
"Syntax error: ORDER BY must come before LIMIT. "
4398+
+ "Please rewrite your query as: ORDER BY ... LIMIT ...",
4399+
ctx);
4400+
}
4401+
}
4402+
43724403
LogicalPlan sort = withSort(inputPlan, sortClauseContext);
43734404
return withLimit(sort, limitClauseContext);
43744405
}

fe/fe-core/src/test/java/org/apache/doris/nereids/parser/NereidsParserTest.java

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,4 +1530,92 @@ public void testUnnest() {
15301530
String sql = "SELECT t.* FROM LATERAL unnest([1,2], ['hi','hello']) WITH ORDINALITY AS t(c1,c2);";
15311531
parsePlan(sql).matches(logicalGenerate().when(plan -> plan.getGenerators().get(0) instanceof Unnest));
15321532
}
1533+
1534+
@Test
1535+
public void testLimitOrderByValidation() {
1536+
NereidsParser nereidsParser = new NereidsParser();
1537+
1538+
// Test 1: Basic error case - LIMIT before ORDER BY (should fail)
1539+
parsePlan("SELECT * FROM test_table LIMIT 3 ORDER BY score DESC")
1540+
.assertThrowsExactly(ParseException.class)
1541+
.assertMessageContains("ORDER BY must come before LIMIT");
1542+
1543+
// Test 2: Error with OFFSET - LIMIT OFFSET before ORDER BY (should fail)
1544+
parsePlan("SELECT * FROM test_table LIMIT 10 OFFSET 5 ORDER BY score DESC")
1545+
.assertThrowsExactly(ParseException.class)
1546+
.assertMessageContains("ORDER BY must come before LIMIT");
1547+
1548+
// Test 3: Error with multiple ORDER BY columns (should fail)
1549+
parsePlan("SELECT * FROM test_table LIMIT 10 ORDER BY a, b DESC")
1550+
.assertThrowsExactly(ParseException.class)
1551+
.assertMessageContains("ORDER BY must come before LIMIT");
1552+
1553+
// Test 4: Error with WHERE clause (should fail)
1554+
parsePlan("SELECT * FROM test_table WHERE id > 0 LIMIT 10 ORDER BY score")
1555+
.assertThrowsExactly(ParseException.class)
1556+
.assertMessageContains("ORDER BY must come before LIMIT");
1557+
1558+
// Test 5: Error with GROUP BY clause (should fail)
1559+
parsePlan("SELECT id, count(*) FROM test_table GROUP BY id LIMIT 5 ORDER BY count(*)")
1560+
.assertThrowsExactly(ParseException.class)
1561+
.assertMessageContains("ORDER BY must come before LIMIT");
1562+
1563+
// Test 6: Correct syntax - ORDER BY before LIMIT (should succeed)
1564+
LogicalPlan plan1 = nereidsParser.parseSingle("SELECT * FROM test_table ORDER BY score DESC LIMIT 3");
1565+
Assertions.assertNotNull(plan1);
1566+
1567+
// Test 7: Correct syntax with OFFSET (should succeed)
1568+
LogicalPlan plan2 = nereidsParser.parseSingle("SELECT * FROM test_table ORDER BY score DESC LIMIT 10 OFFSET 5");
1569+
Assertions.assertNotNull(plan2);
1570+
1571+
// Test 8: Correct syntax with multiple ORDER BY columns (should succeed)
1572+
LogicalPlan plan3 = nereidsParser.parseSingle("SELECT * FROM test_table ORDER BY a, b DESC LIMIT 10");
1573+
Assertions.assertNotNull(plan3);
1574+
1575+
// Test 9: Valid subquery scenario - LIMIT in subquery, ORDER BY in outer query (should succeed)
1576+
LogicalPlan plan4 = nereidsParser.parseSingle(
1577+
"SELECT * FROM (SELECT * FROM test_table LIMIT 10) subq ORDER BY score DESC");
1578+
Assertions.assertNotNull(plan4);
1579+
1580+
// Test 10: Valid CTE scenario - LIMIT in CTE, ORDER BY in main query (should succeed)
1581+
LogicalPlan plan5 = nereidsParser.parseSingle(
1582+
"WITH cte AS (SELECT * FROM test_table LIMIT 10) SELECT * FROM cte ORDER BY score DESC");
1583+
Assertions.assertNotNull(plan5);
1584+
1585+
// Test 11: Valid UNION scenario - LIMIT in one branch, ORDER BY at top level (should succeed)
1586+
LogicalPlan plan6 = nereidsParser.parseSingle(
1587+
"(SELECT * FROM t1 LIMIT 5) UNION ALL (SELECT * FROM t2) ORDER BY a");
1588+
Assertions.assertNotNull(plan6);
1589+
1590+
// Test 12: Correct syntax with WHERE clause (should succeed)
1591+
LogicalPlan plan7 = nereidsParser.parseSingle(
1592+
"SELECT * FROM test_table WHERE id > 0 ORDER BY score LIMIT 10");
1593+
Assertions.assertNotNull(plan7);
1594+
1595+
// Test 13: Correct syntax with GROUP BY clause (should succeed)
1596+
LogicalPlan plan8 = nereidsParser.parseSingle(
1597+
"SELECT id, count(*) FROM test_table GROUP BY id ORDER BY count(*) LIMIT 5");
1598+
Assertions.assertNotNull(plan8);
1599+
1600+
// Test 14: Error with JOIN (should fail)
1601+
parsePlan("SELECT * FROM t1 JOIN t2 ON t1.id = t2.id LIMIT 10 ORDER BY t1.score")
1602+
.assertThrowsExactly(ParseException.class)
1603+
.assertMessageContains("ORDER BY must come before LIMIT");
1604+
1605+
// Test 15: Correct syntax with JOIN (should succeed)
1606+
LogicalPlan plan9 = nereidsParser.parseSingle(
1607+
"SELECT * FROM t1 JOIN t2 ON t1.id = t2.id ORDER BY t1.score LIMIT 10");
1608+
Assertions.assertNotNull(plan9);
1609+
1610+
// Test 16: Valid nested subquery - multiple levels (should succeed)
1611+
LogicalPlan plan10 = nereidsParser.parseSingle(
1612+
"SELECT * FROM (SELECT * FROM (SELECT * FROM test_table LIMIT 5) s1 LIMIT 3) s2 ORDER BY a");
1613+
Assertions.assertNotNull(plan10);
1614+
1615+
// Test 17: Error with aggregate function in ORDER BY (should fail)
1616+
parsePlan("SELECT id, sum(value) FROM test_table GROUP BY id LIMIT 5 ORDER BY sum(value)")
1617+
.assertThrowsExactly(ParseException.class)
1618+
.assertMessageContains("ORDER BY must come before LIMIT");
1619+
}
15331620
}
1621+

0 commit comments

Comments
 (0)