Skip to content

Commit

Permalink
Revert "Workaround SELECT ... ORDER BY (SELECT 1) related bug in Orac…
Browse files Browse the repository at this point in the history
…le's MySQL implementation, that can lead to completely wrong data being returned. (#1850)" (#1871)

(cherry picked from commit 0f66b72)
  • Loading branch information
lauxjpn authored Mar 8, 2024
1 parent d9afafb commit 7bf885a
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -874,28 +874,6 @@ public virtual Expression VisitMySqlBinaryExpression(MySqlBinaryExpression mySql
return mySqlBinaryExpression;
}

protected override Expression VisitOrdering(OrderingExpression orderingExpression)
{
// The base implementation translates this case to `(SELECT 1)`.
// This leads to a bug in Oracle's MySQL, where completely wrong data is being returned under certain conditions.
// This can be reproduced by executing a no-op (or any existing) test of the `ProxyGraphUpdatesMySqlTest+LazyLoading` class (e.g. our own `DummyTest`),
// immediately followed by NorthwindSplitIncludeNoTrackingQueryMySqlTest.Include_collection_OrderBy_empty_list_contains(async: False).
// As a workaround, we just output `1` instead of `(SELECT 1)` for all database versions and types.
if (orderingExpression.Expression is SqlConstantExpression or SqlParameterExpression)
{
Sql.Append("1");

if (!orderingExpression.IsAscending)
{
Sql.Append(" DESC");
}

return orderingExpression;
}

return base.VisitOrdering(orderingExpression);
}

protected virtual Expression VisitJsonTableExpression(MySqlJsonTableExpression jsonTableExpression)
{
// if (jsonTableExpression.ColumnInfos is not { Count: > 0 })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.Extensions.DependencyInjection;
using Pomelo.EntityFrameworkCore.MySql.FunctionalTests.TestUtilities;
using Xunit;

namespace Pomelo.EntityFrameworkCore.MySql.FunctionalTests
{
Expand Down Expand Up @@ -38,12 +37,6 @@ public LazyLoading(ProxyGraphUpdatesWithLazyLoadingMySqlFixture fixture)
{
}

// Used to track down a bug in Oracle's MySQL implementation, related to `SELECT ... ORDER BY (SELECT 1)`.
[Fact]
public void DummyTest()
{
}

protected override bool DoesLazyLoading
=> true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ LIMIT 18446744073709551610 OFFSET @__p_1
SELECT `c`.`CustomerID`, `c`.`Address`, `c`.`City`, `c`.`CompanyName`, `c`.`ContactName`, `c`.`ContactTitle`, `c`.`Country`, `c`.`Fax`, `c`.`Phone`, `c`.`PostalCode`, `c`.`Region`, FALSE AS `c`
FROM `Customers` AS `c`
WHERE `c`.`CustomerID` LIKE 'A%'
ORDER BY 1
ORDER BY (SELECT 1)
LIMIT 18446744073709551610 OFFSET @__p_1
) AS `t`
LEFT JOIN `Orders` AS `o` ON `t`.`CustomerID` = `o`.`CustomerID`
Expand Down Expand Up @@ -2003,7 +2003,7 @@ LIMIT 18446744073709551610 OFFSET @__p_1
SELECT `c`.`CustomerID`, `c`.`Address`, `c`.`City`, `c`.`CompanyName`, `c`.`ContactName`, `c`.`ContactTitle`, `c`.`Country`, `c`.`Fax`, `c`.`Phone`, `c`.`PostalCode`, `c`.`Region`, TRUE AS `c`
FROM `Customers` AS `c`
WHERE `c`.`CustomerID` LIKE 'A%'
ORDER BY 1
ORDER BY (SELECT 1)
LIMIT 18446744073709551610 OFFSET @__p_1
) AS `t`
LEFT JOIN `Orders` AS `o` ON `t`.`CustomerID` = `o`.`CustomerID`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ public override Task Include_collection_with_multiple_conditional_order_by(bool
elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Order>(o => o.OrderDetails)));
}

// Used to track down a bug in Oracle's MySQL implementation, related to `SELECT ... ORDER BY (SELECT 1)`.
public override Task Include_collection_OrderBy_empty_list_contains(bool async)
=> base.Include_collection_OrderBy_empty_list_contains(async);

[ConditionalTheory(Skip = "https://github.com/dotnet/efcore/issues/21202")]
public override Task Include_collection_skip_no_order_by(bool async)
{
Expand Down

0 comments on commit 7bf885a

Please sign in to comment.