1
\$\begingroup\$

I build database called MessageBoard and created StoredProcedure GetHomeView that return data for HomeView. I am completely new in this area.

Stored procedure contains next fields:

Id - Key fied.

ForumId - Field that should be in result query because it is need to know, which forum should be loaded if user will open it.

Category - Name of category.

Forum - Name of forum.

TopicsAmount - Total amount of Topics in Forum.

RepliesCount - Total amount of Posts in Forum.

LastDate - The date of last Post in Forum.

LastUserName - The Name of User that left last Post.

LastTopicName - Last topic where user left last post in forum.

Need advice how to improve structure and performance and keep the same output result. Also I am sure that I made common mistakes help me to find them and to fix.

I tried only CREATE NONCLUSTERED INDEX [TEST_INDEX] ON dbo.Posts (CreationDateTime) INCLUDE (TopicId,UserName) it gave some improvement but not much.

enter image description here

Script generating database structure:

USE [MessageBoard]
GO
/****** Object:  Table [dbo].[Categories]    Script Date: 4/1/2020 01:58:48 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Categories](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](max) NULL,
 CONSTRAINT [PK_Categories] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO
/****** Object:  Table [dbo].[Forums]    Script Date: 4/1/2020 01:58:48 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Forums](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](max) NULL,
    [CategoryId] [bigint] NOT NULL,
 CONSTRAINT [PK_Forums] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO
/****** Object:  Table [dbo].[Posts]    Script Date: 4/1/2020 01:58:48 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Posts](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [Text] [nvarchar](max) NULL,
    [CreationDateTime] [datetime2](7) NOT NULL,
    [TopicId] [bigint] NOT NULL,
    [UserName] [nvarchar](max) NULL,
 CONSTRAINT [PK_Posts] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO
/****** Object:  Table [dbo].[Topics]    Script Date: 4/1/2020 01:58:48 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Topics](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [Text] [nvarchar](max) NULL,
    [CreationDateTime] [datetime2](7) NOT NULL,
    [Author] [nvarchar](max) NULL,
    [ForumId] [bigint] NOT NULL,
 CONSTRAINT [PK_Topics] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO
ALTER TABLE [dbo].[Forums]  WITH CHECK ADD  CONSTRAINT [FK_Forums_Categories_CategoryId] FOREIGN KEY([CategoryId])
REFERENCES [dbo].[Categories] ([Id])
ON DELETE CASCADE
GO
ALTER TABLE [dbo].[Forums] CHECK CONSTRAINT [FK_Forums_Categories_CategoryId]
GO
ALTER TABLE [dbo].[Posts]  WITH CHECK ADD  CONSTRAINT [FK_Posts_Topics_TopicId] FOREIGN KEY([TopicId])
REFERENCES [dbo].[Topics] ([Id])
ON DELETE CASCADE
GO
ALTER TABLE [dbo].[Posts] CHECK CONSTRAINT [FK_Posts_Topics_TopicId]
GO
ALTER TABLE [dbo].[Topics]  WITH CHECK ADD  CONSTRAINT [FK_Topics_Forums_ForumId] FOREIGN KEY([ForumId])
REFERENCES [dbo].[Forums] ([Id])
ON DELETE CASCADE
GO
ALTER TABLE [dbo].[Topics] CHECK CONSTRAINT [FK_Topics_Forums_ForumId]
GO
/****** Object:  StoredProcedure [dbo].[GetRepliesCountByCategories_V21]    Script Date: 4/1/2020 01:58:48 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

Script generating stored procedure:

CREATE PROCEDURE [dbo].[GetHomeView]
AS
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

    -- Insert statements for procedure here
With 

TopicsWithPosts(ForumId,TopicText,PostUserName,TopicId,PostCreationDateTime)
    AS
    (
        SELECT 
        dbo.Topics.ForumId, 
        dbo.Topics.Text, 
        dbo.Posts.UserName,
        dbo.Topics.Id, 
        dbo.Posts.CreationDateTime
        FROM dbo.Topics
        INNER JOIN dbo.Posts on dbo.Topics.Id=dbo.Posts.TopicId
    ),

ForumsWithTopics(ForumId,TopicId)
    AS
    (
        SELECT 
        dbo.Forums.Id,
        dbo.Topics.Id
        FROM
        dbo.Forums 
        INNER JOIN dbo.Topics on dbo.Topics.ForumId = dbo.Forums.Id 
    ),

CountTopicsByForum (ForumId, Amount)
    AS
    (    
        SELECT 
        ForumsWithTopics.ForumId as ForumId, 
        COUNT(distinct ForumsWithTopics.TopicId) as Amount 
        FROM ForumsWithTopics
        GROUP BY 
        ForumsWithTopics.ForumId
    ),

CountPostsByForum (ForumId, Amount)
    AS
    (
        SELECT 
            ForumsWithTopics.ForumId as ForumId, 
            count(dbo.Posts.Id) as Amount
            FROM 
            ForumsWithTopics
            LEFT JOIN dbo.Posts on dbo.Posts.TopicId = ForumsWithTopics.TopicId 
            group by 
            ForumsWithTopics.ForumId
    ),

MaxPostDateInForum(ForumId,LastPostDate)
    AS
    (
    SELECT TopicsWithPosts.ForumId, MAX(TopicsWithPosts.PostCreationDateTime) as LastPostDate
    FROM TopicsWithPosts
    GROUP BY TopicsWithPosts.ForumId
    ),

LastTopicsByDate(ForumId,TopicText,LastPostDate,PostUserName,TopicId,RowNumber)
    AS
    (
    SELECT TopicsWithPosts.ForumId, TopicsWithPosts.TopicText, MaxPostDateInForum.LastPostDate,TopicsWithPosts.PostUserName,TopicsWithPosts.TopicId
    ,row_number() over(partition by TopicsWithPosts.ForumId  order by MaxPostDateInForum.LastPostDate desc) as RowNumber
    FROM 
    TopicsWithPosts 
    INNER JOIN 
    MaxPostDateInForum on TopicsWithPosts.ForumId = MaxPostDateInForum.ForumId and MaxPostDateInForum.LastPostDate=TopicsWithPosts.PostCreationDateTime
    ),

LastUserByDateInForum(ForumId, LastTopicName, LastPostDate,LastUserName,TopicId,RowNumber)
    As
    (
    SELECT 
    LastTopicsByDate.ForumId,
    LastTopicsByDate.TopicText,
    LastTopicsByDate.LastPostDate,
    LastTopicsByDate.PostUserName,
    LastTopicsByDate.TopicId,
    LastTopicsByDate.RowNumber
    FROM LastTopicsByDate
    where LastTopicsByDate.RowNumber = 1
    )

SELECT 
  ROW_NUMBER() over (order by dbo.Categories.Id asc) as Id,
  dbo.Forums.Id as ForumId,
  dbo.Categories.Name as Category, 
  dbo.Forums.Name as Forum, 
  CountTopicsByForum.Amount as TopicsAmount, 
  CountPostsByForum.Amount as RepliesCount,
  LastUserByDateInForum.LastPostDate as LastDate,
  LastUserByDateInForum.LastUserName,
  LastUserByDateInForum.LastTopicName
FROM 
  Categories 
  LEFT JOIN dbo.Forums on dbo.Forums.CategoryId = dbo.Categories.Id 
  LEFT JOIN CountTopicsByForum on CountTopicsByForum.ForumId = dbo.Forums.Id 
  LEFT JOIN CountPostsByForum on dbo.Forums.Id = CountPostsByForum.ForumId
  LEFT JOIN LastUserByDateInForum on dbo.Forums.Id = LastUserByDateInForum.ForumId
GROUP BY 
  dbo.Categories.Id, 
  dbo.Forums.Id,
  dbo.Forums.Name, 
  dbo.Categories.Name, 
  CountTopicsByForum.Amount, 
  CountPostsByForum.Amount,
  LastUserByDateInForum.LastPostDate,
  LastUserByDateInForum.LastUserName,
  LastUserByDateInForum.LastTopicName
  ORDER BY 
   LastUserByDateInForum.LastPostDate DESC

END

--
GO

Script that fill database with data:

DELETE FROM Posts;
DELETE FROM Topics;
DELETE FROM Forums;
DELETE FROM Categories;

SET IDENTITY_INSERT Categories ON;
declare @CategoryNumber int = 1;
while @CategoryNumber <= 10
begin
insert into Categories(Id,Name) values(@CategoryNumber,'Category ' + CONVERT(varchar(MAX), @CategoryNumber))
set @CategoryNumber = @CategoryNumber + 1
end
SET IDENTITY_INSERT Categories OFF;

SET IDENTITY_INSERT Forums ON;
declare @ForumNumber int = 1;
while @ForumNumber <= 50
begin
insert into Forums(Id,Name,CategoryId) values(@ForumNumber,'Test ' + CONVERT(varchar(MAX), @ForumNumber), ((RAND() * 9)+1))
set @ForumNumber = @ForumNumber + 1
end
SET IDENTITY_INSERT Forums OFF;

SET IDENTITY_INSERT Topics ON;
declare @TopicNumber int = 1;
declare @date datetime =  null;
while @TopicNumber <= 10000
begin
SET @date = DATEADD(DAY, ABS(CHECKSUM(NEWID()) % 3650), '2000-01-01')
insert into Topics(Id,Author,CreationDateTime,Text,ForumId) values(@TopicNumber,'Some author '+ CONVERT(varchar(MAX), @TopicNumber),@date,'Topic ' + CONVERT(varchar(MAX), @TopicNumber), ((RAND() * 49)+1))
set @TopicNumber = @TopicNumber + 1
end
SET IDENTITY_INSERT Topics OFF;

SET IDENTITY_INSERT Posts ON;
declare @PostNumber int = 1;
while @PostNumber <= 1000000
begin
SET @date = DATEADD(DAY, ABS(CHECKSUM(NEWID()) % 3650), '2000-01-01')
insert into Posts(Id,Text,CreationDateTime,UserName,TopicId) values(@PostNumber,'Some author ' + CONVERT(varchar(MAX), @PostNumber),@date,'PostNumber ' + CONVERT(varchar(MAX), @PostNumber), ((RAND() * 9999)+1))
set @PostNumber = @PostNumber + 1
end
SET IDENTITY_INSERT Posts OFF;

Need advice how to improve structure and performance and keep the same output result.

I tried only CREATE NONCLUSTERED INDEX [TEST_INDEX] ON dbo.Posts (CreationDateTime) INCLUDE (TopicId,UserName) it gave some improvement but not much.

\$\endgroup\$
  • \$\begingroup\$ It would be very helpful if you posted the related tables as well, and any indexes you created. \$\endgroup\$ – pacmaninbw Apr 1 at 1:11
1
\$\begingroup\$

From what I can gather you are trying to produce a summary view with forum statistics. The code is a bit hard to read and I am not in a position to reproduce your environment right now so I will just give some tips.

In order to analyze and optimize your queries you must absolutely get acquainted with the execution plan. It will tell you how the query parser is processing your SQL, whether it is using indexes etc, and what parts of the code are more expensive.

Indexing: generally you are going to put an index on the fields against which you will be actually searching. For example indexing the user ID in the table of posts makes sense, because searching by user (user ID) is a very common type of request. So when you run a query like select from table where userid = xxx, the database engine can use the index and avoid doing a full table scan to retrieve the results. For an ecommerce site, you will probably index the order date when you frequently count the number of orders within a date range etc.

When you are joining tables, it will also help if the fields being joined are covered by an index.

An index is not an absolute requirement, if you have very few records you won't experience performance problems, but if you have lots of records and join many tables the load is going to increase exponentially.

In terms of data storage some design choices strike me as not so efficient eg:

CREATE TABLE [dbo].[Topics](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [Text] [nvarchar](max) NULL,
    [CreationDateTime] [datetime2](7) NOT NULL,
    [Author] [nvarchar](max) NULL,
    [ForumId] [bigint] NOT NULL,
 CONSTRAINT [PK_Topics] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

The maximum length for a username is usually quite limited (probably < 50 chars) so nvarchar(50) should be more than enough. But it is common to use user IDs. Although there could be one benefit here, if you delete a user from the table of users, you could still retain the original name in the posts hes/she made... for historical purposes.

Even for the post messages [nvarchar](max) does not seem to be justified. I don't think any forum will really allow unlimited post size. In fact even if you did, the webserver will likely deny very large HTTP POST requests (could be a few megabytes).

The performance issues with [nvarchar](max) are not always obvious or immediate but for reference here is one discussion on the matter: Performance Myths : Oversizing string columns

IDs are often int, unless you really expect to have billions of records in your tables... even with lots of table regeneration bigint seems excessive.

As to the stored procedure proper: you are under no obligation to have such a big query with lots of nested SELECT statements. Readability is not that great.

Instead, you could collect the stats in separate statements. And then gather all the values together. Use a temporary table or in-memory table if you want. Since this is a stored procedure and not a regular view you have lots of freedom.

What I would suggest is:

  • rethink your table structure, look at how forums works, download some free forum software like phpbb, freebb or whatever and analyze the code and the database structure
  • break your code in small bits, and evaluate each of them with the help of the execution plan
  • if that is too complicated or too much work, simplify the code by removing some level of detail, until you get acceptable performance - this is one way to determine where the bottleneck is. Again, the execution plan should tell you.

If you still cannot overcome the performance problem, then there is a very old technique: use aggregate tables. These are separate statistical tables that you update every time a post is made, moved or deleted etc. There is more overhead involved, because you must keep them in sync to maintain accurate figures (use transactions to minimize the risk of discrepancy). But this could very well be faster than computing stats on the fly. I would not be surprised if other forums use them.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.