OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.43k stars 2.4k forks source link

Queries: SQL Query Limit results in an exception #7722

Open 4m-world opened 3 years ago

4m-world commented 3 years ago

When adding "limit" to a SQL query on the production system raise an exception with SQL and MySQL database, I tried to overwrite the query by using SQL Top expression but it did not recognize the message

Where a query called from a liquid template.

Note: when I remove the limit from the query it works without problems, except this not the required behaviour

Query v1

SELECT *
  FROM [ContentItemIndex] 
 WHERE [Latest] = 1 
   AND [Published] = 1
   AND [ContentType] = 'BlogPost' 
   AND [ContentItemId] INNER JOIN  IN (SELECT [ContentItemId]
                             FROM [TaxonomyIndex] 
                            WHERE [TermContentItemId] = @termId 
                              AND [ContentType] = 'BlogPost' 
                            LIMIT @limit)
 ORDER BY [PublishedUtc] DESC

Query v2

SELECT DISTINCT C.Id, C.DocumentId, C.ContentItemId, C.ContentItemVersionId, 
C.Latest, C.Published, C.ContentType, C.ModifiedUtc, C.PublishedUtc, 
C.CreatedUtc, C.Owner, C.Author, C.DisplayText
  FROM [ContentItemIndex] AS C INNER JOIN [TaxonomyIndex] AS T ON C.ContentItemId = T.ContentItemId
 WHERE C.[Latest] = 1 
   AND C.[Published] = 1
   AND C.[ContentType] = 'BlogPost'
   AND T.TermContentItemId = @termId
   ORDER BY C.PublishedUTC DESC
 LIMIT @limit
'''

Exception details:

2020-11-23 01:35:11.9695|Default||e8b0552b-47baf56ab4ab99a9.||Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware|ERROR|An unhandled exception has occurred while executing the request. Microsoft.Data.SqlClient.SqlException (0x80131904): The number of rows provided for a TOP or FETCH clauses row count parameter must be an integer. at Microsoft.Data.SqlClient.SqlCommand.<>c.b1640(Task1 result) at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke() at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location where exception was thrown --- at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) --- End of stack trace from previous location where exception was thrown --- at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command) in //Dapper/SqlMapper.Async.cs:line 419 at OrchardCore.Queries.Sql.SqlQuerySource.ExecuteQueryAsync(Query query, IDictionary2 parameters) in C:\projects\orchardcore\src\OrchardCore.Modules\OrchardCore.Queries\Sql\SqlQuerySource.cs:line 74 at OrchardCore.Queries.Liquid.QueryFilter.ProcessAsync(FluidValue input, FilterArguments arguments, TemplateContext ctx) in C:\projects\orchardcore\src\OrchardCore.Modules\OrchardCore.Queries\Liquid\QueryFilter.cs:line 36 at Fluid.Ast.FilterExpression.EvaluateAsync(TemplateContext context) at Fluid.Ast.AssignStatement.WriteToAsync(TextWriter writer, TextEncoder encoder, TemplateContext context) at Fluid.Ast.ForStatement.WriteToAsync(TextWriter writer, TextEncoder encoder, TemplateContext context) at Fluid.Ast.IfStatement.WriteToAsync(TextWriter writer, TextEncoder encoder, TemplateContext context) at Fluid.BaseFluidTemplate1.Awaited(ValueTask1 task, TextWriter writer, TextEncoder encoder, TemplateContext context, List1 statements, Int32 startIndex) at OrchardCore.DisplayManagement.Liquid.LiquidViewTemplateExtensions.RenderAsync(LiquidViewTemplate template, TextWriter writer, TextEncoder encoder, LiquidTemplateContext context, Object model, Action`1 scopeAction) in C:\projects\orchardcore\src\OrchardCore\OrchardCore.DisplayManagement.Liquid\LiquidViewTemplate.cs:line 239 at OrchardCore.Templates.Services.TemplatesShapeBindingResolver.<>cDisplayClass8_0.<b__0>d.MoveNext() in C:\projects\orchardcore\src\OrchardCore.Modules\OrchardCore.Templates\Services\TemplatesShapeBindingResolver.cs:line 78

4m-world commented 3 years ago

Update: the issue take place when calling the query using Queries extension despite the limit parameter set inside the original query or passed as a parameter, while on the other hand, the query run without any problem from the query window.

Skrypt commented 3 years ago

The number of rows provided for a TOP or FETCH clauses row count parameter must be an integer. You either did not pass any value or a string.

4m-world commented 3 years ago

@Skrypt I did pass an integer and I tried to set the limit within the query itself not passing it and it had the same exception, now after updating OC to latest build 15520 the issue resolved for a value stored in the query, however, passed value is not recognized as an integer and raises the same exception.

It worth to mention, that this only took place on the production server and using MS SQL database, where my local coding is done against sqlite.

Skrypt commented 3 years ago

Ok keeping this issue open for investigation.

4m-world commented 3 years ago

after further investigation, the issue takes place on MSSQL server version 14.0.2027.2 with server collation: SQL_Latin1_General_CP1_CI_AS

emrahtokalak commented 2 years ago

We encountered this problem when we imported a query that worked fine on SQLITE into a structure using MSSQL.

the part that caused the error in our query; It was the use of LIMIT @pageSize: 8 OFFSET @offset: 0

If it works properly with MSSQL; LIMIT {{ pageSize }} OFFSET {{ offset }}

sebastienros commented 2 years ago

@emrahtokalak First the SQL you type is db agnostic. We parse it with our own parser and then translate it to the destination provider (sqlite, mssql, ...).

When you write LIMIT @pageSize: 8 OFFSET @offset: 0 I wonder if the space in @pageSize: 8 is the issue. Maybe the parser is lost there.

Could you share the resulting generated SQL for both SQLite and MSSQL if you remove the space? It is displayed somewhere in the screen when the query is executed.

emrahtokalak commented 2 years ago

Hi @sebastienros,

Even though I deleted the space, the result did not change. I ran it like "LIMIT @pagesize:8 OFFSET @offset:0".

Let me share all the details below;

Query causing problem with MSSQL;

SELECT i.DocumentId FROM ContentItemIndex i INNER JOIN ContainedPartIndex cpi on cpi.DocumentId = i.DocumentId INNER JOIN DateTimeFieldIndex di on di.ContentItemId = i.ContentItemId WHERE i.Published = true AND cpi.ListContentItemId = @listId ORDER BY di.DateTime DESC LIMIT @pageSize:8 OFFSET @offset:0


DB: MSSQL - "LIMIT @pageSize:8 OFFSET @offset:0" - RawQuery result; (I caught it from OrchardCore.Queries.Sql.SqlQuerySource class) - Error

SELECT [i].[DocumentId] FROM [ContentItemIndex] AS i INNER JOIN [ContainedPartIndex] AS cpi ON [cpi].[DocumentId] = [i].[DocumentId] INNER JOIN [DateTimeFieldIndex] AS di ON [di].[ContentItemId] = [i].[ContentItemId] WHERE [i].[Published] = 1 AND [cpi].[ListContentItemId] = @listId ORDER BY [di].[DateTime] DESC OFFSET @offset ROWS FETCH NEXT @pageSize ROWS ONLY;


DB: MSSQL - "LIMIT {{ pageSize }} OFFSET {{ offset }}" - RawQuery result; (I caught it from OrchardCore.Queries.Sql.SqlQuerySource class) - Successful

SELECT [i].[DocumentId] FROM [ContentItemIndex] AS i INNER JOIN [ContainedPartIndex] AS cpi ON [cpi].[DocumentId] = [i].[DocumentId] INNER JOIN [DateTimeFieldIndex] AS di ON [di].[ContentItemId] = [i].[ContentItemId] WHERE [i].[Published] = 1 AND [cpi].[ListContentItemId] = @listId ORDER BY [di].[DateTime] DESC OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY;


DB: SQLite - "LIMIT @pageSize:8 OFFSET @offset:0" - RawQuery result; (I caught it from OrchardCore.Queries.Sql.SqlQuerySource class) - Successful

SELECT [i].[DocumentId] FROM [ContentItemIndex] AS i INNER JOIN [ContainedPartIndex] AS cpi ON [cpi].[DocumentId] = [i].[DocumentId] INNER JOIN [DateTimeFieldIndex] AS di ON [di].[ContentItemId] = [i].[ContentItemId] WHERE [i].[Published] = 1 AND [cpi].[ListContentItemId] = @listId ORDER BY [di].[DateTime] DESC LIMIT @pageSize OFFSET @offset;


DB: SQLite - "LIMIT {{ pageSize }} OFFSET {{ offset }}" - RawQuery result; (I caught it from OrchardCore.Queries.Sql.SqlQuerySource class) - Successful

SELECT [i].[DocumentId] FROM [ContentItemIndex] AS i INNER JOIN [ContainedPartIndex] AS cpi ON [cpi].[DocumentId] = [i].[DocumentId] INNER JOIN [DateTimeFieldIndex] AS di ON [di].[ContentItemId] = [i].[ContentItemId] WHERE [i].[Published] = 1 AND [cpi].[ListContentItemId] = @listId ORDER BY [di].[DateTime] DESC LIMIT 10 OFFSET 0;

Skrypt commented 2 years ago

Thanks for your tests. Looks like a MSSQL Dialect issue. Need to see if the issue now is in OC or in YesSQL.

sebastienros commented 2 years ago

Thanks @emrahtokalak it looks like the resulting MSSQL query fails because it doesn't handle either limit, offset or both as a parameter. We need to add a unit tests in yessql to prove that.

Here is a mitigation:

SELECT i.DocumentId FROM ContentItemIndex i INNER JOIN ContainedPartIndex cpi on cpi.DocumentId = i.DocumentId INNER JOIN DateTimeFieldIndex di on di.ContentItemId = i.ContentItemId WHERE i.Published = true AND cpi.ListContentItemId = @listId 
ORDER BY di.DateTime DESC 
{% if pageSize > 0 %}LIMIT {{ pageSize }} {% endif %}
{% if offset > 0 %}OFFSET {{ offset }} {% endif %}

This way the parameters won't be used in the SQL query, but the actual values. I still think we should investigate by filing an issue in yessql.