U4-9195 - A user with the login already exists after - U4-8837 Change paging query for SQL server 2012+ to be more efficient

Created by Jan van Helvoort 16 Nov 2016, 14:10:00 Updated by Shannon Deminick 17 Nov 2016, 16:06:08

Tags: Unscheduled

Is duplicated by: U4-9198

Relates to: U4-9190

Umbraco 7.5.4 and 7.5.5

If you want to change some properties of a user, you get the following error: ERROR umbraco.cms.presentation.user.EditUser - Exception System.Exception: A user with the login 'foo@foo.com' already exists at umbraco.BusinessLogic.User.set_LoginName(String value) at umbraco.cms.presentation.user.EditUser.SaveUser_Click(Object sender, EventArgs e)

After diging in the source, we found the following commit: {{U4-8837 Change paging query for SQL server 2012+ to be more efficient}} where {{BuildSqlDbSpecificPagingQuery}} wraps the orginal query inside a peta poco paging query. string.Format("SELECT * FROM (SELECT ROW_NUMBER() OVER ({0}) peta_rn, {1}) peta_paged WHERE peta_rn>@{2} AND peta_rn<=@{3}", sqlOrderBy == null ? "ORDER BY (SELECT NULL)" : sqlOrderBy, sqlSelectRemoved, args.Length, args.Length + 1);

If we update a user, the stack trace: {{user.cs - EnsureUniqueLoginName}} -> {{user.cs - User.getAllByLoginName}} -> {{uservservice.cs - FindByUsername -> repository.GetPagedResultsByQuery}} -> {{UserRepository.cs - GetPagedResultsByQuery}}.

In this method on line number 342 returns the wrong id. It returns the {{peta_rn}} instead of the value of the orginal query if the type is {{int}}. var page = Database.Page(pageIndex + 1, pageSize, idsQuery);

Comments

Sebastiaan Janssen 16 Nov 2016, 14:56:41

@janvanhelvoort What SQL server version are you on?


Stephan 16 Nov 2016, 15:12:49

Running latest dev-v7 on Sql Express 2016 and I cannot reproduce (I understand one just has to go to the backoffice users section and save any user, correct?) - looking into code to try to see what's going on.


Erik Thijssen 16 Nov 2016, 15:13:26

Hi Stephan, our developement server is SQL Server 2008 R2


Erik Thijssen 16 Nov 2016, 15:26:00

Hi Stephan,

We hit the "databaseType == DBType.SqlServer" so the code will use the original "sql" parameter by adding the peta_rn (ROW_NUMBER()) to the resultset and out parameter "sqlPage". If we run profiler on SQLServer we see that the query being used in de fetch function (sqlPage) will return 2 fields (peta_rn, id) instead of 1 field (using e.g. SqlServerCE). The first value is always 1 because the search for existing user always return 1 row. The second field contains the correct UmbracoUser ID. The "sqlPage" is used to Fetch the result to an generic item of int. This causes the error, because it will fetch the rownummer (first field) instead of the UmbracoUser ID.

internal virtual void BuildSqlDbSpecificPagingQuery(DBType databaseType, long skip, long take, string sql, string sqlSelectRemoved, string sqlOrderBy, ref object[] args, out string sqlPage) { if (databaseType == DBType.SqlServer || databaseType == DBType.Oracle) { sqlSelectRemoved = rxOrderBy.Replace(sqlSelectRemoved, ""); if (rxDistinct.IsMatch(sqlSelectRemoved)) { sqlSelectRemoved = "peta_inner.* FROM (SELECT " + sqlSelectRemoved + ") peta_inner"; } sqlPage = string.Format("SELECT * FROM (SELECT ROW_NUMBER() OVER ({0}) peta_rn, {1}) peta_paged WHERE peta_rn>@{2} AND peta_rn<=@{3}", sqlOrderBy == null ? "ORDER BY (SELECT NULL)" : sqlOrderBy, sqlSelectRemoved, args.Length, args.Length + 1); args = args.Concat(new object[] { skip, skip + take }).ToArray(); }

Let us know if you need more input


Stephan 16 Nov 2016, 15:31:59

OK, have been able to reproduce (by disabling the version detection and forcing the use of Sql 2008 code). Consistently fails on 2008. Looking into it.


Stephan 16 Nov 2016, 16:09:15

OK, actually the issue is not linked to U4-8837 but to U4-8698 which introduces the Page<int> calls which do not work on SqlServer <=2008. Was obviously tested on >2008 and SqlCE. Now need to figure out whether PetaPoco Page<T> can be fixed or replaced by something else...


Stephan 16 Nov 2016, 17:08:28

PR https://github.com/umbraco/Umbraco-CMS/pull/1624

Fixes PetaPoco Page<int>. Test on Sql Server 2008 (or tweak UmbracoDatabase to pretend it's 2008), should work, all tests should be green, nothing else should fail (eg list views, stuff that use pagination).


Erik Thijssen 17 Nov 2016, 08:47:56

Thanks @zpqrtbnk !!


Jamie Attwood 17 Nov 2016, 16:04:03

Just to chime in, I have this exact issue on two instances running sql 2008. Thanks for fixing!


Shannon Deminick 17 Nov 2016, 16:05:56

tested with paging, paging with custom properties, etc... using the <= 2008 syntax and is working, merging now


Priority: Critical

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.5.4

Due in version: 7.5.5

Sprint: Sprint 46

Story Points:

Cycle: