Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add random() order support #16964

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public SqlGrammar() : base(false)

// Create Index.
orderList.Rule = MakePlusRule(orderList, comma, orderMember);
orderMember.Rule = Id + orderDirOptional;
orderMember.Rule = Id + (orderDirOptional | "(" + functionArguments + ")");
orderDirOptional.Rule = Empty | "ASC" | "DESC";

// Select stmt.
Expand Down
26 changes: 21 additions & 5 deletions src/OrchardCore.Modules/OrchardCore.Queries/Sql/SqlParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,20 +246,36 @@ private void EvaluateOrderClause(ParseTreeNode parseTreeNode)
var idList = parseTreeNode.ChildNodes[2];

_modes.Push(FormattingModes.SelectClause);

for (var i = 0; i < idList.ChildNodes.Count; i++)
{
var id = idList.ChildNodes[i].ChildNodes[0];

if (i > 0)
{
_builder.Append(", ");
}

var id = idList.ChildNodes[i].ChildNodes[0];

// RANDOM() is a special case where we need to use the dialect's random function.
if (id.ChildNodes[0].Token?.ValueString == "RANDOM")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could work in SQLite & Postgres only, RAND is the one that works in SQL Server and MySQL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_builder.Append(_dialect.RandomOrderByClause);

But maybe you forgot how sql queries are managed in this feature. Call me if you want a quick refresher.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I revisited YesSQL and after I looked to _dialect.RandomOrderByClause it's fine for me

{
var funArgs = idList.ChildNodes[i].ChildNodes[1].ChildNodes[0];

// "RANDOM" + {funArgs} + no arguments?
if (funArgs.Term.Name == "funArgs" && funArgs.ChildNodes.Count == 0)
{
_builder.Append(_dialect.RandomOrderByClause);
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
continue;
continue;

}
}

EvaluateId(id);

if (idList.ChildNodes[i].ChildNodes[1].ChildNodes.Count > 0)
var orderDirOpt = idList.ChildNodes[i].ChildNodes[1].ChildNodes[0];

if (orderDirOpt.Term.Name == "orderDirOpt" && orderDirOpt.ChildNodes.Count > 0)
{
_builder.Append(' ').Append(idList.ChildNodes[i].ChildNodes[1].ChildNodes[0].Term.Name);
_builder.Append(' ').Append(orderDirOpt.ChildNodes[0].Term.Name);
}
}

Expand Down Expand Up @@ -805,7 +821,7 @@ private void EvaluateOverClauseOptional(ParseTreeNode overClauseOpt)
var orderMember = orderList.ChildNodes[i];
var id = orderMember.ChildNodes[0];
EvaluateSelectId(id);
var orderDirOpt = orderMember.ChildNodes[1];
var orderDirOpt = orderMember.ChildNodes[1].ChildNodes[0];
if (orderDirOpt.ChildNodes.Count > 0)
{
_builder.Append(' ').Append(orderDirOpt.ChildNodes[0].Term.Name);
Expand Down
6 changes: 6 additions & 0 deletions src/docs/reference/modules/Queries/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,12 @@ The SQL parser is also able to convert some specific functions to the intended d
| `year(_date_)` | Returns the years part of a date. |
| `now()` | Returns current date time (utc). |

Order By clauses can also use the `random()` function (case insensitive) to order results randomly, e.g.,

```
SELECT * FROM ContentItemIndex ORDER BY random()
```

## Scripting

The following JavaScript functions are available with this module.
Expand Down
11 changes: 11 additions & 0 deletions test/OrchardCore.Tests/Orchard.Queries/SqlParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,15 @@ public void ShouldParseSubquery(string sql, string expectedSql)
Assert.True(result);
Assert.Equal(expectedSql, FormatSql(rawQuery));
}

[Theory]
[InlineData("select a order by RANDOM()", "SELECT [a] ORDER BY newid();")]
[InlineData("select a order by random()", "SELECT [a] ORDER BY newid();")]
[InlineData("select a order by RANDOM", "SELECT [a] ORDER BY [RANDOM];")]
public void ShouldOrderByRandom(string sql, string expectedSql)
{
var result = SqlParser.TryParse(sql, _schema, _defaultDialect, _defaultTablePrefix, null, out var rawQuery, out var errors);
Assert.True(result);
Assert.Equal(expectedSql, FormatSql(rawQuery));
Comment on lines +254 to +256
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var result = SqlParser.TryParse(sql, _schema, _defaultDialect, _defaultTablePrefix, null, out var rawQuery, out var errors);
Assert.True(result);
Assert.Equal(expectedSql, FormatSql(rawQuery));
// Arrange & Act
var result = SqlParser.TryParse(sql, _schema, _defaultDialect, _defaultTablePrefix, null, out var rawQuery, out var errors);
// Assert
Assert.True(result);
Assert.Equal(expectedSql, FormatSql(rawQuery));

}
}
Loading