SP works but seems slow. Is there a better more effecient way to write this?

For your question: No.

But you can optimize the query execution. Take a look at the actual execution plan. And add proper indicies to support your query.

Depending on the cardinalities also the correct choice of the clustered index is important.

Possibly the right join.

And might like to try select distinct rather than group by if there is no aggregation.

What indexes do you have ?

Hmmmm....

So basically you want to find questions not answered

When c.[PeopleID] IS NULL, then the inner joins wont return data either....

Maybe the query can be simplified :

	SELECT DISTINCT 
      q.[RegistrationQuestionID],
	  q.[RegistrationQuestion]
      
  FROM [dbo].[RegistrationQuestion] q  
  WHERE NOT EXISTS (SELECT NULL from [dbo].[RegistrationAnswerPeopleCluster] c where c.[RegistrationQuestionID] = q.[RegistrationQuestionID] and c.[Active] = 1)  
  ORDER BY q.[RegistrationQuestionID] ASC

Select allOpen in new window

Or left join and continue to check c.[PeopleID] IS NULL

Am I oversimplifying ?

Albeit slow, what non-null columns are being returned ?

Any sample / test data to do some analysis with ?

Hi Folks

Mark:
PeopleID can be null and the joins will work as it isn't the key for that table.


I'm wondering if all the foreign key columns in RegistrationAnswerPeopleCluster have appropriate indexes. Generally on recent versions of SQL the execution plan will alert you to the possibility of missing indexes. And now that SQL Server Management Studio (SSMS) is a free download and no longer tied to a particular version, I suggest using the latest version of that.

HTH
  David

Here is the C# model of this returning data.  I know you are not necessarily a code programmer but I think it's pretty easy to read and understand.

public class RegistrationQuestion
        {

            public string RegistrationQuestionType { get; set; }

            public string RegistrationQuestionReturnType { get; set; }

            public int QuestionID { get; set; }

            public string Question { get; set; }

            public List<RegistrationAnswer> RegistrationAnswers { get; set; }


        }

        public class RegistrationAnswer
        {

            public int AnswerID { get; set; }

            public string Answer { get; set; }


        }

Select allOpen in new window

Can you post Actual Execution Plan when you execute this store procedure? To give you any suggestion to improve this query, we need to understand what are causing performance issue such as
- Data volume size
- Missing indexes
...

@David,

Yes, I understand that, and the reason for the RIGHT join in conjunction with C.People is null appears (to me) to be asking for "questions not participated in by people"

Might not have expressed it the right way (a little obscure) before, and really mean that the inner joins seem to be redundant to the cause, of the end goal which still seems to be questions without participants.  

And asked for clarification. Which I will ask (in a different way)...

What do you want to see as a result of the query - what is the business objective ?

Can we see some sample data and expected results ?

I was over-thinking it.  This is 2 seconds faster - can this be improved?

IF EXISTS (SELECT * FROM SYSOBJECTS WHERE ID = OBJECT_ID(N'[dbo].[usp_GetRegistrationQuestions]') AND OBJECTPROPERTY(id, N'IsProcedure') = 1)
DROP PROCEDURE [dbo].[usp_GetRegistrationQuestions];
GO
CREATE PROCEDURE [dbo].[usp_GetRegistrationQuestions]
AS
BEGIN	
	SELECT 
	  t.[RegistrationQuestionType],
      r.[RegistrationQuestionReturnType],
      q.[RegistrationQuestionID],
	  q.[RegistrationQuestion]
      
  FROM [dbo].[RegistrationAnswerPeopleCluster] c
  JOIN [dbo].[RegistrationQuestionType] t ON c.[RegistrationQuestionTypeID] = t.[RegistrationQuestionTypeID]
  JOIN [dbo].[RegistrationQuestionReturnType] r ON c.[RegistrationQuestionReturnTypeID] = r.[RegistrationQuestionReturnTypeID]
  JOIN [dbo].[RegistrationQuestion] q ON c.[RegistrationQuestionID] = q.[RegistrationQuestionID]  
  WHERE c.[PeopleID] IS NULL AND c.[Active] = 1
  GROUP BY t.[RegistrationQuestionType], r.[RegistrationQuestionReturnType], q.[RegistrationQuestionID], q.[RegistrationQuestion]
  ORDER BY q.[RegistrationQuestionID] ASC
END
GO

Select allOpen in new window

Again: Take a look at the actual execution plan. And add proper indicies to support your query.

We cannot really tell, what you can do, without seeing the actual execution plan..

Thanks