Design encouraging FK to non-unique/non-PK

Question

My current design is makes me want to create a FK to a non-unique/non-PK. Is this a code smell and/or does my overall design make sense?

CREATE TABLE [User].[ConnectionRequest]
(
    [ConnectionRequestId] INT NOT NULL PRIMARY KEY IDENTITY,
    [Requestor] INT NOT NULL, -- FK to internal user table
    [Target] INT NOT NULL, -- FK to ConnectionRequestTarget table
    [Status] INT NOT NULL, -- FK to status enum descriptions
    [Created] DATETIME2(2) NOT NULL DEFAULT SYSUTCDATETIME(),

    CONSTRAINT [FK_Name_Truncated_For_Space] 
      FOREIGN KEY ([Target]) 
      REFERENCES [User].[ConnectionRequestTarget]([ConnectionRequestTargetId]),     
    CONSTRAINT [Unique_ConnectionRequest_Target_Status] UNIQUE ([Target], [Status])
    -- Other FK's removed for brevity
)

CREATE TABLE [User].[ConnectionRequestTarget]
(
    [ConnectionRequestTargetId] INT NOT NULL PRIMARY KEY IDENTITY,
    [InternalUserId] INT NULL, -- FK to internal user table, can be updated
    [ReferralId] INT NULL, -- FK to referral table
    [RequestCreated] DATETIME2(2) NOT NULL DEFAULT SYSUTCDATETIME(),
    -- FK's removed for brevity
);

CREATE TABLE [User].[PushSent]
(
    [PushSentId] INT NOT NULL PRIMARY KEY IDENTITY,
    [MatchingToken] UNIQUEIDENTIFIER NOT NULL,
    [PushDeviceId] INT NOT NULL,
    [Category] INT NOT NULL,
    [MonitoringId] INT NULL,
    [ConnectionRequestTargetId] INT NULL,
    [ThresholdForNextAlert] DATETIME2(2) NULL,
    [Scheduled] DATETIME2(2) NOT NULL DEFAULT SYSUTCDATETIME(),
    [Sent] DATETIME2(2) NULL,

    CONSTRAINT [FK_PushSent_ConnectionRequestTargetId_to_ConnectionRequest_Target] 
       FOREIGN KEY ([ConnectionRequestTargetId]) 
       REFERENCES [User].[ConnectionRequest]([Target]), -- This doesn't work
    -- Other FK's and check constraints removed for brevity
);

The ConnectionRequest table contains a history of the connection requests and their status updates. I normalized the target of the connection request into its own table so the target details wouldn't be duplicated with each status update. For example:

-- ConnectionRequest table sample data
ConnectionRequestId | Requestor | Target | Status | Created
        1                 1         1        1      2015-10-3 10:31:23.93 -- Request Pending
        2                 2         2        1      2015-10-3 10:31:25.89 -- Request Pending
        3                 1         3        1      2015-10-3 11:45:12.49 -- Request Pending
        4                 1         1        3      2015-10-3 12:45:12.49 -- Request Accepted
        5                 1         4        1      2015-10-3 12:45:12.49 -- Request Pending

-- ConnectionRequestTarget table sample data
ConnectionRequestTargetId | InternalUserId | ReferralId | RequestCreated
          1                       3             NULL      2015-10-3 10:31:23.93 -- Internal
          2                       3             NULL      2015-10-3 10:31:25.89 -- Internal
          3                      NULL            1        2015-10-3 11:45:12.49 -- External
          4                       4              2        2015-10-3 12:45:12.49 -- Updated

As you can see connection status updates are handled by inserting a new row into the ConnectionRequest table. The (Target, Status) UNIQUE constraint in the ConnectionRequest table is there to ensure a Target doesn't get multiple entries for the same status. Furthermore a new ConnectionRequestTarget is generated for each request even if the user being targeted is the same. The InternalUserId column in the ConnectionRequestTarget table can be updated if the target wasn't a user, and then creates an account.

This worked great until a requirement came in to periodically prompt users with a push notification if they haven't acted^{(accept/deny)} on a connection request. Due to that request I want to put a FK into the PushSent table which will let me track the number of pushes sent to a target.

I can't set the FK to the Target column in the ConnectionRequest table because it is non-unique by design. I know I can put the Status flag into the PushSent table with the Target, or handle this with a UDF and a check constraint, but those options feel a bit dirty. I could also specify a FK to the ConnectionRequestTarget table directly from the PushSent table but that could open the door for associating pushes to an orphaned target.

In truth it gets a bit more complicated than this because a Target of a ConnectionRequest can be a current member, or a potential member, but the core requirement to track the count of pushes/contacts to a Target remains.

Answer 1

It seems to me you have an entity Target against which you may have an active ConnectionRequest. It would appear that the Status of a request will be changed from time to time as the request is processed. In that case, that field should not be made part of any key or unique constraint. Only stable (unchanging) fields should be part of a key. In this case, it would appear that the Created field would be perfect.

However, I don't see why you need to create a unique constraint at all. You already assign a unique PK to each request, why don't you have ConnectionRequestTargetId refer to ConnectionRequest( ConnectionRequestID )? This would safely tie each PushSent to one specific request.

Answer 2

Based on the comment by ypercube:

Why does the Target need to be in the PushSent table? Can't you just use a ConnectionRequestId column instead (which would be an FK to ConnectionRequest)?

I realized I was approaching this problem wrong. I had focused on the Target relationship until I developed tunnel vision.

In the end I decided against holding a FK to the ConnectionRequest table or ConnectionRequestTarget table in my PushSent table. I ended up creating the table below instead to record each contact attempt:

CREATE TABLE [Log].[ContactAttempt]
(
    [ContactAttemptId] INT NOT NULL PRIMARY KEY,
    [Method] INT NOT NULL, -- FK to enum description table, 1 = SMS, 2 = Email, 3 = Push
    [ConnectionRequestId] INT NOT NULL, -- FK to ConnectionRequest table
    [Created] DATETIME2(2) NOT NULL DEFAULT SYSUTCDATETIME()
    -- FK definitions removed for brevity
)

Using that table I could get the information I needed using the following query:

DECLARE @Now DATETIME2(2) = CAST(SYSUTCDATETIME() AS DATETIME2(2));
;WITH [NotificationCount_CTE] AS
(
    SELECT
        [CR].[ConnectionRequestId]
        , DATEADD(DAY, 1, [CR].[Created]) AS [FirstCutoff]
        , DATEADD(DAY, 4, [CR].[Created]) AS [SecondCutoff]
        , DATEADD(DAY, 7, [CR].[Created]) AS [ThirdCutoff]
        , CASE -- This could probably be improved
            WHEN MAX([CA].[Created]) > DATEADD(DAY, 7, [CR].[Created]) THEN 4 
            ELSE CASE WHEN MAX([CA].[Created]) > DATEADD(DAY, 4, [CR].[Created]) THEN 3
            ELSE CASE WHEN MAX([CA].[Created]) > DATEADD(DAY, 1, [CR].[Created]) THEN 2
            ELSE 1 END END
          END AS [ContactCount] -- Use a CASE statements so early counts aren't 
                                -- included in the filter below
    FROM [User].[ConnectionRequest] [CR]
        LEFT JOIN [Log].[ContactAttempt] [CA] ON 
           [CA].[ConnectionRequestId] = [CR].[ConnectionRequestId]
    WHERE
        [CR].[Status] = 1 -- Pending status.
    GROUP BY
        [CR].[ConnectionRequestId]
        , [CR].[Created]
)
SELECT
    [PDA].[DeviceToken]
    , ISNULL([PDA].[PushDeviceId], 0) AS [PushDevice]
    , [PDA].[PushSystem]
    , CASE WHEN [CCR].[InternalUserId] IS NULL THEN [CCR].[ContactInfo] 
          ELSE ISNULL([SU].[Email], [U_T].[PhoneNumber]) 
      END AS [ContactInfo] -- External users invititation contact. 
                           -- Internal users use email first, phone number second.
    , [U_R].[Fullname] -- Name of person who initiated contact request
    , [U_R].[DisplayName] -- Fall back if they didn't provide a full name
    , [PDA].[CurrentPushCount]
    , [CR].[ConnectionRequestId] AS [RequestId]
    , [CCR].[ReferralCode] -- Code for tracking external invites via a link
FROM [User].[ConnectionRequest] [CR]
    INNER JOIN [User].[CurrentConnectionRequestStatus] [CCR] ON 
       [CCR].[Target] = [CR].[Target]
    INNER JOIN [NotificationCount_CTE] [NC] ON 
       [NC].[ConnectionRequestId] = [CR].[ConnectionRequestId]
    LEFT JOIN [User].[CurrentPushDeviceAssociation] [PDA] WITH (NOEXPAND) ON 
       [PDA].[UserId] = [CCR].[InternalUserId]
    LEFT JOIN [User].[User] [U_R] ON 
       [U_R].[UserId] = [CCR].[Requestor]
    LEFT JOIN [User].[User] [U_T] ON 
       [U_T].[UserId] = [CCR].[InternalUserId]
    LEFT JOIN [Security].[User] [SU] ON 
       [SU].[UserId] = [CCR].[InternalUserId]
WHERE
    [CCR].[Status] = 1 -- We only remind if the request is still pending
    AND
    (
        (@Now > [NC].[FirstCutoff] AND [NC].[ContactCount] = 1)
        OR
        (@Now > [NC].[SecondCutoff] AND [NC].[ContactCount] = 2)
        OR
        (@Now > [NC].[ThirdCutoff] AND [NC].[ContactCount] = 3)
    );

The CurrentConnectionRequestStatus reference is a view that shows the current status for the connection, and denormalizes some of the associated data.

The key insight besides the one provided by ypercube was that I really didn't want/need a true push/contact count. What I really wanted was to see if any contact attempt was made at certain intervals. By combining the various contact methods into one table I could easily track what methods of contact had been attempted, and when they were attempted.

---

To explicitly answer my original question:

1. Is this a code smell?
   • Yes, this was a code smell caused by my tunnel vision. Once my eyes were opened I was able to eliminate the apparent conflict.
2. Does my overall design make sense?
   • The overall design needed to be tweaked to cleanly reflect the requirements.