0

I'm working on a ASP.NET project using Identity (2.2). One of the things I was trying to put together was datagrid where the Users and Roles would be loaded onto one datagrid where CRUD commands could be run. I had no problem storing the select statement:

CREATE PROCEDURE [dbo].SelectUsersRoles
AS
    SET NOCOUNT ON;

    SELECT 
        p.Id, p.Email, p.PhoneNumber, p.LockoutEnabled, p.UserName, 
        p.AspNetUserRole, AspNetRoles.Name 
    FROM 
        AspNetUsers AS p 
    INNER JOIN 
        AspNetUserRoles ON p.Id = AspNetUserRoles.UserId 
    INNER JOIN 
        AspNetRoles ON AspNetUserRoles.RoleId = AspNetRoles.Id
GO

And this selects the data and I can load the grid just fine. After reviewing this question, and this one, I was left with the idea that the correct update code should have been:

[EDIT, ran as stored procedure updated below, same issue]

CREATE PROCEDURE [dbo].UpdateUserRoles
(
    @Email nvarchar(256),
    @UserName nvarchar(256),
    @PhoneNumber nvarchar(MAX),
    @LockoutEnabled bit,
    @original_ID nvarchar(128)
)
AS
    SET NOCOUNT ON;

    UPDATE p
    SET p.Email = @Email, 
        p.Username = @UserName, 
        p.PhoneNumber = @PhoneNumber, 
        p.LockoutEnabled = @LockoutEnabled
    FROM [AspNetUsers] AS p
    INNER JOIN [AspNetUserRoles] AS r ON  p.ID = r.UserID
    INNER JOIN [AspNetRoles] AS b ON r.RoleID = b.ID
    WHERE p.ID = @original_ID
    GO

If I try to use the Query Builder, I notice VS'13 automatically adds a CROSS JOIN statement to it: [and generally makes it less readable]

UPDATE p
SET p.Email = @Email, p.UserName = @UserName, 
    p.PhoneNumber = @PhoneNumber, p.LockoutEnabled = @LockoutEnabled
FROM            
    AspNetUsers AS p 
INNER JOIN
    AspNetUserRoles AS r ON p.Id = r.UserId 
INNER JOIN
    AspNetRoles AS b ON r.RoleId = b.Id 
CROSS JOIN
    p
WHERE
    (p.Id = @original_ID)

And this keeps throwing the error that 'p' is an invalid object when I try to run the query.

Is there something simple I'm missing, or would this be better to split into consecutive update commands? The end goal is to update both the user information (ie phone number) along with the role assigned to the user.

Just to have the DB set up in question, I think you could start any new MVC/Web Forms template from Visual Studio, make a few users and put in some roles.

Community
  • 1
  • 1
Atl LED
  • 656
  • 2
  • 8
  • 31
  • 1
    Can you use a stored procedure instead of keeping your code in the application? It provides much better separation to get the data in a data layer and out of the application layer. It also fixes these kinds of issues easily. – Sean Lange Aug 24 '15 at 16:32
  • @SeanLange To my knowledge, I am trying to use a stored procedure. I haven't gotten passed trying to add the UPDATE quarry to the stored SELECT quarry (which works). I suppose I could draft it in it's stored form as the 1st one... but that won't change the actual quarry part will it ? – Atl LED Aug 24 '15 at 16:35
  • Well the first code snippet is a procedure. The second one (the update statement) is not part of the first procedure and doesn't appear to be a procedure at all. But without some context it is hard to say. I highly doubt that studio would add a cross join to your query inside a stored procedure. That just seems a bit strange. – Sean Lange Aug 24 '15 at 17:31
  • @SeanLange It seems the Quarry Builder is doing it once I paste in the code, and ether click "ok" or test it in execute. If I forgo that step and just store the procedure, then when I call it I get the same "p" is an invalid object error. I'm going to try not renaming any tables, but I can't see that as wrong? – Atl LED Aug 24 '15 at 17:40
  • 1
    Don't use the query builder. That is the beginning of the problem. Then you need to remove the alias in the list of columns you are updating. (Update p set Email = @Email...) – Sean Lange Aug 24 '15 at 17:48
  • I would also question your datatypes. Why is phone number varchar(max). Do you really need that much storage?? Even with country codes you should be safe with about 20 characters for any number on the planet. Also there is no need to use nvarchar for email. You do not need to extended character set to handle any valid email character. – Sean Lange Aug 24 '15 at 17:50
  • @SeanLange Lesson learned on Query builder, I will try to get off that crutch. RE:datatypes, I haven't bothered to change them from any defaults that load with the template. Still playing around with getting it to function as intended, and this will only ever be on an inward facing net/sever. – Atl LED Aug 24 '15 at 17:58

1 Answers1

2

Please remove the table aliases on the left side of the = directly under the UPDATE P SET. This is implied and is the cause of your errors.

UPDATE p SET
    Email = @Email, 
    Username = @UserName, 
    PhoneNumber = @PhoneNumber, 
    LockoutEnabled = @LockoutEnabled
FROM [AspNetUsers] AS p
    INNER JOIN [AspNetUserRoles] AS r
        ON  p.ID = r.UserID
    INNER JOIN [AspNetRoles] AS b
        ON r.RoleID = b.ID
WHERE p.ID = @original_ID

Note: There is no reason for the joins - assuming these are because of your query builder. You just need below in your stored proc.

UPDATE AspNetUsers SET
    Email = @Email, 
    Username = @UserName, 
    PhoneNumber = @PhoneNumber, 
    LockoutEnabled = @LockoutEnabled
WHERE ID = @original_ID
Jason W
  • 13,026
  • 3
  • 31
  • 62
  • Well at the moment my plan was to just try and get valid syntax on updating the [AspNetUsers] but my goal would be also to update [AspNetUserRoles].RoleID, though I'm displaying [AspNetRoles].Name – Atl LED Aug 24 '15 at 17:55
  • 2
    You'll need a second UPDATE statement to update the RoleID since you can't update 2 separate tables in the same query. You may consider wrapping the 2 UPDATE statements in a transaction since they are related to the same "update" being performed, and I would assume you wouldn't want to have 1 update save but the other not save due to a system error (deadlock victim, network, etc). But usually you just INSERT/DELETE the AspNetUserRoles table instead of trying to update that - just always seemed like not a great idea. – Jason W Aug 24 '15 at 18:03
  • Any particular reason or source you can point to as why you shouldn't update the UserRoles table? It seems like I would have to expand this to two steps, first INSERT the new value then delete the old one? – Atl LED Aug 24 '15 at 18:12
  • 2
    The primary key is a composite key in UserRoles comprised of the UserId and RoleId. I've always tried to avoid directly updating any part of the primary key of a table for many reasons. One of which that is practical here is since it is part of your clustered index, and as such, you are re-organizing the underlying pages in the table increasing the level of page-level fragmentation and over time with large data will slow down these very common queries. One good write-up: http://dba.stackexchange.com/questions/42276/sql-server-clustered-index-high-fragmentation – Jason W Aug 24 '15 at 18:26