Error with single SQL query

Question

I am trying to write an SQL query for my database that will retrieve all of a particular users events. If they are marked as recurring, I want them to show up weekly for a period of 52 weeks after its start date. Is there a better way of doing this? My current code seems quite clunky.

My tables are structured as follows :

   event (
    event_id INT NOT NULL AUTO_INCREMENT, (PK)
    title VARCHAR(80) NOT NULL,
    description VARCHAR(200),
    event_start DATETIME,
    event_end DATETIME,
    group_id INT NOT NULL,
    recurring BOOLEAN
);

   Users{
    UserID (PK)
    Username
   }

   Groups{
    GroupID (PK)
    GroupName
   }

   Membership{
    UserID (FK)
    GroupID (FK)
   }

Here is the query I have at the moment, it will not currently work, can anyone please help me with it?

 var result = db.Query(
            @"SELECT e.event_id, e.title, e.description, e.event_start, e.event_end, e.group_id, e.recurring
            FROM   event e
            JOIN   Membership m ON m.GroupID = e.group_id
            WHERE  e.recurring
            AND    m.UserID = ?
            AND    e.event_start >= ?
            AND    e.event_end <= ?
            UNION ALL
            SELECT e.event_id, e.title, e.description, DATEADD(week, w.weeks, e.event_start),
                   DATEADD(week, w.weeks, e.event_end), e.group_id, e.recurring
            FROM   event e
            JOIN   Membership m ON m.GroupID = e.group_id,
            (SELECT row_number() OVER (ORDER BY Object_ID) AS weeks
                    FROM SYS.OBJECTS) AS w
            WHERE  NOT e.recurring
            AND    m.user_id = ?
            AND    e.event_start >= ?
            AND    e.event_end <= ?", username, start, end, username, start, end
        );

Can anyone help me adapt my code so that it selects all events that are not recurring and it selects recurring events at the same time for a period of 52 weeks?

As a side note, remember that a year does not have 52 weeks. — Albin Sunnanbo, Mar 09 '12 at 21:40
I've seen systems that gets another couple of ugly hacks in blind panic every time the current year does not have 52 weeks. — Albin Sunnanbo, Mar 09 '12 at 21:44
How does your query's actual results differ from your desired results? — Neil, Mar 09 '12 at 21:53
I would suggest a consistent naming style. Don't change it from one table to the next. If you camel-case your column names, stick with that. I prefer having all lower case identifiers, but that is influenced by PostgreSQL. — Erwin Brandstetter, Mar 10 '12 at 02:13
You supply a `username` now where the query expects a `UserID`. Either join in the table `Users` additionally (`JOIN User u ON u.UserID = m.UserID` and check `AND u.Username = ?` or supply a `UserID`. — Erwin Brandstetter, Mar 10 '12 at 02:23

Erwin Brandstetter · Accepted Answer · 2012-03-10T02:00:44.767

1

Could look like this:

SELECT e.event_id, e.title, e.description, e.start_time, e.end_time
     , e.group_id, e.recurring
FROM   Events e
JOIN   Membership m ON m.groupid = e.group_id
WHERE  e.recurring = 0 
AND    m.user_id = ?
AND    e.start_time >= ?
AND    e.end_time <= ?

UNION ALL
SELECT e.event_ID, e.title, e.description
      ,DATEADD(week, w.weeks, e.start_time) -- AS event_start
      ,DATEADD(week, w.weeks, e.end_time)   -- AS event_end
      ,e.group_id, e.recurring
FROM   Events e
JOIN   Membership m ON m.groupid = e.group_id
      ,(SELECT row_number() OVER (ORDER BY Object_ID) AS weeks
        FROM SYS.OBJECTS) AS w
WHERE  e.recurring = 1
AND    m.user_id = ?
AND    e.start_time >= ?
AND    e.end_time <= ?

Use UNION ALL instead of UNION.
event_start and event_end instead of start_time and end_time (??) I changed that.
I missed that, too, at first: user_id is in table Membership, so you need to JOIN that in, if you want to use it in the WHERE clause. Foreign key columns seems to be group_id / groupid. (I would unify the spelling!)
You had SELECT * in the first part of the query. It's better to spell out the column names in such a query, or later changes to the underlying table will break it in surprising ways.
My first draft had an auxiliary function generate_series() to generate the set of numbers 1-52 from here, but your original solution is better, so I reverted that change.

edited Mar 10 '12 at 02:00

answered Mar 09 '12 at 21:52

Erwin Brandstetter

605,456
145
1,078
1,228

Where should I add the function generate_series in my code in order to integrate this? Many thanks for the response - it's just went a little bit over my head when you shows the auxiliary function! :) – Simon Kiely Mar 09 '12 at 22:03
@SimonKiely: Sorry, that auxiliary function is actually not necessary. Your origninal solution is simpler. I reverted that change. – Erwin Brandstetter Mar 09 '12 at 22:08
Brandstetter , Hi, I have tried to integrate this code into my system and get the error : "There was an error parsing the query. [ Token line number = 4,Token line offset = 13,Token in error = AND ]] " – Simon Kiely Mar 10 '12 at 01:13
I've updated my code above to show exactly what I am running - username is a string, start and end are datetimes. – Simon Kiely Mar 10 '12 at 01:15
I think the error is that the user_id column is in another table than the event table being queried. – Simon Kiely Mar 10 '12 at 01:23
@SimonKiely: `user_id` seems to be a column in table `Membership`. Obviously, we need to join in that table to use the column in the WHERE clause. See amended answer. – Erwin Brandstetter Mar 10 '12 at 01:27
Thank you very much for your answer! I did not know that was possible! I have updated some column-names and edited my answer above to reflect what I am currently running, which is returning the error - "There was an error parsing the query. [ Token line number = 5,Token line offset = 17,Token in error = AND". Would you be able to see if you can see any mistake in this please? I have looked thoroughly and am stumped! – Simon Kiely Mar 10 '12 at 01:49
@SimonKiely: My bad again. I am so used to having a boolean type like in other RDBMS that I was fooled by your table definition claiming `BOOLEAN`. [SQL Server does not have a boolean type](http://stackoverflow.com/questions/7170688/sql-server-boolean-literal). So you need to check against 0 / 1 like you already had. Fixed that. – Erwin Brandstetter Mar 10 '12 at 02:04

score 1 · Answer 2 · answered Mar 09 '12 at 21:55

Try this. You used event_start and event_stop in your SQL, but they were not defined in the table Events. The only thing you had mixed up was start_time <= @CurrentTime AND end_time >= @CurrentTime. I don't think your code is clunky. UNION ALL performs better than UNION (UNION works like DISTINCT: first ORDER BY then remove duplicates).

DROP TABLE #Events
CREATE TABLE #Events (
  event_id  INT NOT NULL,
  title VARCHAR(80) NOT NULL,
  description   VARCHAR(200),
  start_time    DATETIME,
  end_time  DATETIME,
  group_id  INT NOT NULL,
  recurring bit
);

INSERT INTO #Events(event_id, title, description, start_time, end_time, group_id, recurring) VALUES
(1, 'meeting1', 'meeting1', '20000101', '20000102', 1, 0),
(2, 'meeting2', 'meeting2', '20120309', '20120310', 1, 0),
(3, 'meeting3', 'meeting3', '20120311', '20120312', 1, 0),
(10, 'meeting10', 'meeting10', '20000101', '20000102', 1, 1),
(20, 'meeting20', 'meeting20', '20120309', '20120310', 1, 1),
(30, 'meeting30', 'meeting30', '20120311', '20120312', 1, 1)

DECLARE @CurrentTime DATETIME
SET @CurrentTime = GETDATE()

SELECT event_id, 
    title,
    description, 
    start_time, 
    end_time, 
    group_id, 
    recurring
FROM #Events 
WHERE recurring = 0 AND 
--  user_id = ? AND 
  start_time <= @CurrentTime AND 
  end_time >= @CurrentTime

UNION ALL

SELECT event_ID, 
  title, 
  description, 
  DATEADD(WEEK, Interval, start_time) [event_start], 
  DATEADD(WEEK, Interval, end_time) [event_end], 
  group_id, 
  recurring
FROM #Events,
  (SELECT ROW_NUMBER() OVER(ORDER BY Object_ID) [Interval]
  FROM SYS.OBJECTS) as T 
WHERE recurring = 1 AND 
  Interval <= 52 AND 
--  user_id = ? AND 
  start_time <= @CurrentTime AND 
  end_time >= @CurrentTime
;

Hi, thank you for the response. It is very helpful. The issue I am having at the moment is that the user_id column is contained within the membership table. I would like to only retrieve the events which are related to users where the group_id contains their user_id - so the membership table will contain a group_id and several user_id's, showing all the users who are the member of a specific group. If the group_id is used in the event, then I would only like to retrieve the event for one of these users. — Simon Kiely, Mar 10 '12 at 01:28
>If the group_id is used in the event, then I would only like to retrieve the event for one of these users.< The Group_ID is NOT NULL, it will always be used in the event. — Wim, Mar 10 '12 at 08:13

Error with single SQL query

2 Answers2