0

I am trying to write a UDF that is used by a check constraint but seem to keep running into an issue.

When having a complete empty table the following results in 1

declare @a float = (SELECT max(amount) FROM Bid WHERE auctionId = 1)
if(@a is null) select 1
else select 0

But when I try to implement similar logic into an UDF it seems to return 0 every time running it against an empty table.

CREATE FUNCTION dbo.fn_ck_newBidIsHigher(@auction int, @bid numeric(8,2))
RETURNS bit
BEGIN
    DECLARE @currentHighestBid float = (SELECT max(amount) FROM Bid WHERE auctionId = @auction)

    IF((@bid > @currentHighestBid) OR (@currentHighestBid is null))
    BEGIN
        RETURN 1
    END

    RETURN 0
END
GO

've been looking at it for over an hour now (maybe that's the problem), but I can't figure out where it's going wrong.

I am calling the function in the check constraint as follows:

ALTER TABLE Bid ADD CONSTRAINT ck_New_Bid_Must_Be_Higher_Than_Previous_Bid CHECK(dbo.fn_ck_newBidIsHigher(auctionId, amount) = 1)
Piet Hein
  • 184
  • 2
  • 16
  • Did you think that this condition might be coming as true? `@bid > @currentHighestBid` – Niranjan Rajawat May 01 '18 at 12:16
  • If that would result in true it should return 1. The check gets triggered every time so it seems it returns 0. – Piet Hein May 01 '18 at 12:18
  • The `CHECK` logically takes place after an `INSERT` or `UPDATE` has taken place (but just before it would be rolled back because the check failed). The table is never empty when the `CHECK` runs, and an empty table has no need to run `CHECK`s. The logic you're applying here seems tailor made for a trigger, not a `CHECK` -- the telltale sign is the fact that you're talking about "new" bids, when a `CHECK` should make sense on a table in any particular state, and cannot refer to "new" rows. (Even the trigger has to take care to distinguish between old and new rows, but at least it *can*.) – Jeroen Mostert May 01 '18 at 12:25
  • Since you're comparing `@bid` to `@currentHighestBid`, I'm curious why you aren't using the same data type for each. Comparing numeric to float introduces what seems to be unnecessary volatility. Have you tested what happens if you declare `@currentHighestBid` as numeric(8,2) instead? This isn't right on point, but it demonstrates one issue with floats: https://stackoverflow.com/questions/10330087/ms-sql-float-decimal-comparison-problems – Eric Brandt May 01 '18 at 12:34
  • @JeroenMostert, the check at the end need to check if a bid is valid. It's valid when for example the bid is 5 dollars and the current bid is between 50 and 100 dollars. Should this be done with an after update check? – Piet Hein May 01 '18 at 12:36
  • @EricBrandt thanks to point that out. Completely missed it (does not solve the problem but indeed they should be the same datatype). – Piet Hein May 01 '18 at 12:38
  • You have: `IF((@bid > @currentHighestBid) OR (@currentHighestBid is null))` meaning, if `@currentHighestBid` is null, return 1. What were you expecting? – FDavidov May 01 '18 at 12:49
  • @FDavidov that it returns 1 and passes the check. – Piet Hein May 01 '18 at 13:05
  • It *could* be done in a `CHECK` as well, but only if you had a way of excluding the new row (for example, based on an identity value or a time stamp). The problem currently is that `@bid > @currentHighestBid` is always false, because the `SELECT MAX(..)` will also select the just inserted/updated `@bid` value. In a trigger, you have access to `inserted` and `deleted` for easier checking. If you changed the condition to `>=`, you should find your check passes -- but then it allows bids that aren't higher than the current bids, which is probably not what you want. – Jeroen Mostert May 01 '18 at 13:07
  • Well. You could also just include `AND amount <> @bid`, I suppose, assuming bids are unique in the first place. That doesn't work in general for all tables, but in this case it should. – Jeroen Mostert May 01 '18 at 13:08

1 Answers1

0

Instead of using an IF, how about using a CASE? When using a CASE you can have more than 2 possible results. F.e. 1 or 0 or NULL, instead of only 1 or 0.

Also, it's safer to compare values of the same type.
Just to avoid any unexpected side-effects from the implicit cast/convert before the comparison.

CREATE FUNCTION dbo.fn_ck_newBidIsHigher(@auction int, @bid numeric(8,2))
RETURNS bit
BEGIN

    DECLARE @currentHighestBid numeric(8,2) = (SELECT max(amount) FROM Bid WHERE auctionId = @auction);

    DECLARE @result bit = (
      case 
      when @bid > @currentHighestBid then 1 
      when @bid <= @currentHighestBid then 0 
      end);

RETURN @result;
END
LukStorms
  • 28,916
  • 5
  • 31
  • 45