0

I have 3 related tables representing objects: clients, users and leads. Users have type 'User' or 'Admin', can create leads and serve clients. In schema terms, Users.Client references Clients.Id in a many-to-one relationship, and Leads.CreatedBy references Users.Username in a many-to-one relationship. Sample schema:

CREATE TABLE Clients (
  Id INT IDENTITY PRIMARY KEY,
  Name VARCHAR(32) NOT NULL
);

CREATE TABLE Users (
  Id INT,
  Username VARCHAR(32) NOT NULL,
  Type VARCHAR(8) NOT NULL CHECK (Type IN ('Admin', 'User')),
  Client INT NOT NULL,
  PRIMARY KEY (Username),
  UNIQUE (id),
  FOREIGN KEY (Client) REFERENCES Clients (Id)
);

CREATE TABLE Leads (
  Id INT IDENTITY PRIMARY KEY,
  Name VARCHAR(64),
  Company VARCHAR(64),
  Profession VARCHAR(64),
  CreatedBy VARCHAR(32) NOT NULL,
  FOREIGN KEY (CreatedBy) REFERENCES Users (Username)
);

I'm writing a query to show a user their leads. Users of type 'User' should only be able to view only the leads they've created. Users of type 'Admin' should be able to see all leads for their client (but not for other clients). What query will fetch rows from the Leads table according to these restrictions? I've checked other Q&As, but I couldn't figure out how to apply them to the situation described above.

I tried the following:

SELECT * 
  FROM Leads 
  WHERE createdby IN (
    CASE 
      WHEN (SELECT type 
              FROM users 
              WHERE username='Sathar'
           )='Admin' 
        THEN (
          SELECT username 
            FROM users 
            WHERE client=(
              SELECT client 
                FROM users 
                WHERE username='Sathar'
        )   )
      ELSE 'Sathar'
    END
  )

However, it generates the error:

Subquery returned more than 1 value. This is not permitted when the subquery follows =, !=, <, <= , >, >= or when the subquery is used as an expression.

To round out the example, some sample data:

SET IDENTITY_INSERT Clients ON;

INSERT INTO Clients (Id, Name)
  VALUES
(1, 'IDM'),
(2, 'FooCo')
;

SET IDENTITY_INSERT Clients OFF;

INSERT INTO Users (Id, Username, Type, Client)
  VALUES
(1, 'Sathar', 'Admin', 1),
(2, 'bafh', 'Admin', 1),
(3, 'fred', 'User', 1),
(4, 'bloggs', 'User', 1),
(5, 'jadmin', 'Admin', 2),
(6, 'juser', 'User', 2)
;


INSERT INTO Leads (Name, Company, Profession, CreatedBy)
  VALUES
('A. Person', 'team lead', 'A Co', 'Sathar'),
('A. Parrot', 'team mascot', 'B Co', 'Sathar'),

('Alice Adams', 'analyst', 'C Co', 'juser'),
('"Bob" Dobbs', 'Drilling Equipment Salesman', 'D Co', 'juser'),
('Carol Kent', 'consultant', 'E Co', 'juser'),

('John Q. Employee', 'employee', 'F Co', 'fred'),
('Jane Q. Employee', 'employee', 'G Co', 'fred'),

('Bob Howard', 'Detached Special Secretary', 'Capital Laundry Services', 'jadmin')
;

All the above is available as a live example.

Without the CASE expression, the query generates no errors, but doesn't follow all the restrictions (all leads for a client are returned for users of type User):

SELECT * 
  FROM Leads 
  WHERE createdby IN (
    SELECT username 
      FROM users 
      WHERE client=(
        SELECT client 
          FROM users 
          WHERE username='fred'
      )
  ) 

This can be seen in the results shown in another live example.

outis
  • 75,655
  • 22
  • 151
  • 221
Taqi Gates
  • 25
  • 6
  • 1
    A select statement returns a set of rows. That could be no rows, one row, or many rows. Unless the `username` column on your `users` table is constrained to be unique, your `select type from users where username='Sathar')` subquery can return more than one row. But you then compare that *set* of rows against a single scalar value, "Admin". The same problem applies to your other subqueries. A case expression has to return a single value, but you are returning the result of a select that might have multiple rows. – allmhuran Jul 07 '22 at 12:25
  • hint : use `top 1` instead type and client will be one and only one for a user!! – Nikhil S Jul 07 '22 at 12:36
  • 2
    A simple search will find many discussions of this error and how to address it. If you want a solution specific to your environment, then you need to post a script or fiddle that contains DDL and sample data to reproduce your problem. Your query has a suspicious amount of complexity and can probably be rewritten to address the problem, to be easier to understand, and to be more efficient. It is highly suspicious that you use a specific literal multiple times in your query. – SMor Jul 07 '22 at 12:46
  • 1
    Expanding on SMor's comment, since SQL includes data definition, a [mcve] for an [SQL question](//meta.stackoverflow.com/q/333952/90527) should include [DDL](//en.wikipedia.org/wiki/Data_definition_language) statements for sample tables and [DML](//en.wikipedia.org/wiki/Data_manipulation_language) statements for sample data. Desired results don't need to be presented as sample code, as results are the output of code and not code themselves. – outis Jul 07 '22 at 13:04
  • 2
    This is a prime situation to use joins, rather than subqueries. – outis Jul 07 '22 at 13:06
  • 3
    Just dumping a `TOP 1` on the query is poor advice unless you have an explicit `ORDER BY` too, @nikhilsugandh . – Thom A Jul 07 '22 at 13:07
  • Please show us your table design for table Leads, I am reasonable sure you could solve your problem much more simple with a join instead of this complex subqueries – GuidoG Jul 07 '22 at 13:12
  • Using the username as a foreign key can lead to data inconsistency. Instead, the `users` table should have a [surrogate key](https://en.wikipedia.org/wiki/Surrogate_key), which can be used as a foreign key in other tables. – outis Jul 07 '22 at 13:16
  • 1
    @outis Why should it have a surrogate key? Slapping surrogate keys on tables where there's a viable natural key is not something I would do as a matter of course. if the `username` can be modified then there's an argument for a surrogate, but we don't know the OP's business rules, so I don't think it's reasonable to assume that `username` is mutable and provide general advice based on that assumption without stating it explicitly. – allmhuran Jul 07 '22 at 13:21
  • Please supply a [mcve] containing your table schema otherwise it's impossible to answer what the intention is here – Charlieface Jul 07 '22 at 14:04
  • I have attached my database structure. Here are relations Leads.createdby=Users.username Clients.id = Users.client – Taqi Gates Jul 07 '22 at 17:07
  • [Images](//meta.stackoverflow.com/q/285551/90527) should not be used for textual data, such as schema. As previously mentioned, you should use DDL & DML statements for the example. Samples should be *minimal*. Please read the previously linked articles and apply the guidelines. – outis Jul 07 '22 at 21:45
  • @allmhuran: Concretely: because typos, and because business rules change. While you're right that surrogate keys aren't always necessary in general, usernames in particular are problematic as primary keys. This likely has something to do with the reason for usernames: they're a label for humans to distinguish accounts (usernames are themselves surrogates, in a way), but the consequences are subtle. – outis Jul 07 '22 at 21:48
  • When it comes to storing an enumerated type in a column, see: "[Does SQL Server 2005 have an equivalent to MySql's ENUM data type?](//stackoverflow.com/q/262802/90527)", "[SQL Server equivalent to MySQL enum data type?](//stackoverflow.com/q/1434298/90527)", "[Create enum in SQL Server](https://stackoverflow.com/a/52223873/90527)", "[8 Reasons Why MySQL's ENUM Data Type Is Evil](//komlenic.com/244/8-reasons-why-mysqls-enum-data-type-is-evil/)" – outis Jul 17 '22 at 18:50

3 Answers3

1

This should be done as an if else not as a case, you can do it like:

if (SELECT type FROM users WHERE username='[the username]') = 'Admin'
begin
    --The records you allow for admin to see for example
    (SELECT * 
    FROM Leads where createdby in 
    (
        select username 
        from users where client=(select client from users where username='Sathar')
    ))
end
else
begin
    --The records you allow for non admin to see for example
    (SELECT * 
    FROM Leads 
    WHERE createdby = 'Sathar')
end

Or really the 'admin' one should be done with joins, but same idea.

The reason this should be done as if else is because case is a expression not a statement (like in other languages switch case), while if else is a conditional statement, and it would be better design when you have separate selects for each type of user, and it will help you not to create errors.

Shmiel
  • 1,201
  • 10
  • 25
  • select * from Leads where createdby in (if (select type from users where username='Sathar')='Admin' begin (select username from users where client=(select client from users where username='Sathar')) end else begin 'Sathar' end) this gives me an error Incorrect syntax near the keyword 'if'. – Taqi Gates Jul 07 '22 at 20:01
  • @TaqiGates See my edit. – Shmiel Jul 07 '22 at 20:25
  • works fine but this can be used as a where clause? so that I can write the same for all the tables... – Taqi Gates Jul 07 '22 at 22:11
  • No, and see my edit with explanation why (I think) it would be better to use `if`. – Shmiel Jul 07 '22 at 22:19
  • You could probably use this in dynamic sql or stored procedure, so would be able to use this for multiple tables. – Shmiel Jul 08 '22 at 13:04
0

Subqueries are an older technique. While it might be possible to write a query that produces the desired results with a subquery, using JOINs are in general a better match for the relational model, as JOINs are the foundational operation for relating data between tables.

The basics of JOINs come from the relationships defined within the schema: foreign keys. A query will generally have a JOIN for each foreign key.

Starting Point

For the query, there are two relevant pieces of data: the user who created the lead, and the user viewing the leads. This suggests the Users table should be consulted twice: once for the lead creator, and once for the viewer (who should at least belong to the same client as the creator). This gives a generic query, which can then be modified to meet the conditions for various situations (such as the restrictions in the question):

SELECT Leads.*
  FROM Leads
    JOIN Users AS creator
      ON Leads.CreatedBy = creator.Username
    JOIN Users AS viewer
      ON viewer.Client = creator.Client
  WHERE viewer.Username = @user

Restrictions in Join Conditions

Since the restrictions (and query) are about associating the viewer with leads that they can view, a natural approach is to express the restrictions in the join condition for viewer. In this context, the restrictions are:

  1. The creator and the viewer are the same, or
  2. The viewer is of type 'Admin' and belongs to the same client as the creator.

In SQL:

   creator.Id = viewer.Id
OR (    viewer.type = 'Admin'
    AND viewer.Client = creator.Client)

Since we're redefining how rows are associated, these conditions replace the general join conditions (note the general conditions are part of these conditions), giving:

SELECT Leads.*
  FROM Leads
    JOIN Users AS creator
      ON Leads.CreatedBy = creator.Username
    JOIN Users AS viewer
      ON    viewer.Id = creator.Id
         OR (    viewer.type = 'Admin'
             AND viewer.Client = creator.Client)
  WHERE viewer.Username = @user
;

You can see the results of this query for both an admin and an unprivileged user.

Restrictions in Filter

Conditions could go in an ON clause or the WHERE clause (before JOIN syntax was introduced, both join conditions and filter conditions went in WHERE). If, conceptually speaking, conditions apply to which rows associate, they generally should go in the join conditions. If conditions instead conceptually restrict the results of a (possibly intermediate) table, they should go in the WHERE clause. To my way of thinking, the conditions in this case are about row association, and should thus go in the ON clause. However, it's useful to look at the other option, especially as examining the query execution plan for all the options may show one performs better (the various live examples linked to in this answer show execution plans for the example data by way of illustration, but the execution plans for the production data should be examined before making decisions for the production system).

Adding the restriction conditions to the general query is a simple matter, starting with the list of restrictions. Since the "same client" restriction is already present in the join condition, simply drop that part of the restrictions, giving:

  1. The creator and the viewer are the same, or
  2. The viewer is of type 'Admin'

In SQL:

   creator.Id = viewer.Id
OR viewer.type = 'Admin'

Next, add the conditions to the WHERE clause. Since the viewer selection criteria should still apply, the conditions are combined with an AND:

SELECT Leads.*
  FROM Leads
    JOIN Users AS creator
      ON Leads.CreatedBy = creator.Username
    JOIN Users AS viewer
      ON viewer.Client = creator.Client
  WHERE viewer.Username = @user
        AND (   creator.Id = viewer.Id
             OR viewer.type = 'Admin')

The results (with execution plans) can be viewed for both user types in a live example.

outis
  • 75,655
  • 22
  • 151
  • 221
-2

Use this :

select * from Leads where createdby in (case when (select TOP 1 type from 
users where username='Sathar')='Admin' then (select username from users 
where client=(select TOP 1 client from users where username='Sathar' )) else 
'Sathar' end);

select type from users where username='Sathar' will give only one type so better use TOP 1 for it and same goes for select client from users where username='Sathar'

Nikhil S
  • 3,786
  • 4
  • 18
  • 32
  • gives me the same error. Folks, username and client are primary keys, there is no possibility of duplicatation. – Taqi Gates Jul 07 '22 at 12:38
  • @TaqiGates `Client` is a candidate key in your `users` table? That seems unlikely. And even if it *were* true, your query wouldn't make any sense, because it says "go and get the clients for this particular user, and then tell me the users associated with that client". If client and user are both unique, then there is only one client per user, which means that the user returned by the expression *must be* the same as the user you put in as the predicate in the first place, so there'd be no reason to run a query at all. – allmhuran Jul 07 '22 at 12:45
  • Sorry, I mean in users table 'username' is primary key. 'client' is not primary key, it is a foreign key from clients table. A client has many usernames with type 'user' and 'Admin' – Taqi Gates Jul 07 '22 at 12:49
  • @TaqiGates Right, so if you say "get me the usernames associated with a given client", then that query can return more than one row. – allmhuran Jul 07 '22 at 12:50
  • If `top 1` worked then the result would be an arbitrary row, which is usually not what the business logic requires. But moreover this answer is not going to solve the problem because the fundamental misunderstanding is around `case`. The OP is attempting a construct of the form `where MyValue in (case when P1 then {set} else {other set} end)`. But a `case` expression cannot return a set. – allmhuran Jul 07 '22 at 12:59
  • 2
    A `TOP` without an `ORDER BY` is a sure sign of a flaw. This means that the data engine is free to return what ever arbitrary row(s) it wants, and the row(s) could be different every time you run said query. If you are using `TOP` you need to ensure the query has an `ORDER BY` so that you get consistent and reliable results. I do not suggest this is the answer here, as the query *could* behave differently *every* time it is run. In fact, the 2 identical subqueries could return different values within the same query; which could give some *very* odd behaviour. – Thom A Jul 07 '22 at 13:04
  • SELECT * FROM Leads where createdby in (select username from users where client=(select client from users where username='Sathar')) This is working perfectly, even this returns multiple rows. As I mentioned in my question, the query works without CASE. with case it thorw error. – Taqi Gates Jul 07 '22 at 15:55
  • @TaqiGates As I have already said in three places, the result of a case expression cannot be a set. You cannot construct a case statement of the form `case when some_condition then (select some set of rows) ... else ... `. So a fortiori you cannot write `select ... where something in (case when some_condition then (select some set of rows) else ...`. The result of `case` must be a scalar. You have a `case` where the first result is not a scalar. – allmhuran Jul 07 '22 at 17:37