Monday, February 20, 2012

Is my UPDATE statement buggy?

QA has told me that on two occassions the following sp updated all rows to
the same @.BillerID value. Could this happen? Is my TSQL buggy? This is the
last step of a lengthy data migration process. The SPs are executed by a
C#/.Net forms app.
DDL statements for all three tables follow (Payer and PayerXref exist in
seperate database on the same server). Thanks in advance.
--The SP
/*
CREATED BY: KEVIN WILLIAMS
CREATED ON: 10/24/2005
NOTES:
will set the BillerId fields in PayerDB.dbo.Payer and PayerDB.dbo.Account to
the BillerId
passed by the user.
These fields now hold the primary key of the V3 tables
*/
CREATE PROCEDURE dbo.usp_MigratePayerService_BillerId
@.CREDID BIGINT,
@.BILLERID BIGINT
AS
DECLARE @.errnum INT
SET NOCOUNT ON
----
UPDATE PayerDB.dbo.Payer
SET BillerId = @.BILLERID
FROM Migration.dbo.PayerXref X
JOIN PayerDB.dbo.Payer P
ON(X.AccountUserID = P.BillerId)
WHERE X.BillerId = @.BILLERID
----
SET @.errnum = @.@.ERROR
IF @.errnum<>0
BEGIN
GOTO BAILOUT
END
----
UPDATE PayerDB.dbo.Account
SET BillerId = @.BILLERID
FROM Migration.dbo.AccountXref X
JOIN PayerDB.dbo.Account A
ON(X.AccountID_TP3 = A.BillerId)
WHERE x.BillerId = @.BILLERID
----
SET @.errnum = @.@.ERROR
----
BAILOUT:
RETURN @.errnum
--End of SP
--Create statements for tables
CREATE TABLE dbo.Payer
(PayerId bigint IDENTITY (1, 1) NOT NULL ,
Identifier varchar (50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
BillerId bigint NOT NULL)
--other fields removed for sake of space
CREATE TABLE dbo.PayerXref
(AccountUserID bigint NOT NULL ,
PayerId bigint NULL ,
BillerId bigint NOT NULL ,
Migrated bit NOT NULL CONSTRAINT DF_PayerXref_Migrated DEFAULT (0),
ErrorDescription varchar (4000) COLLATE Latin1_General_CI_AS NULL)instead of:
UPDATE PayerDB.dbo.Payer
SET BillerId = @.BILLERID
FROM Migration.dbo.PayerXref X
JOIN PayerDB.dbo.Payer P
ON(X.AccountUserID = P.BillerId)
WHERE X.BillerId = @.BILLERID
use the alias, P:
UPDATE P
SET BillerId = @.BILLERID
FROM Migration.dbo.PayerXref X
JOIN PayerDB.dbo.Payer P
ON(X.AccountUserID = P.BillerId)
WHERE X.BillerId = @.BILLERID
kevin wrote:
> QA has told me that on two occassions the following sp updated all rows t
o
> the same @.BillerID value. Could this happen? Is my TSQL buggy? This is t
he
> last step of a lengthy data migration process. The SPs are executed by a
> C#/.Net forms app.
> DDL statements for all three tables follow (Payer and PayerXref exist in
> seperate database on the same server). Thanks in advance.
> --The SP
> /*
> CREATED BY: KEVIN WILLIAMS
> CREATED ON: 10/24/2005
> NOTES:
> will set the BillerId fields in PayerDB.dbo.Payer and PayerDB.dbo.Account
to
> the BillerId
> passed by the user.
> These fields now hold the primary key of the V3 tables
> */
> CREATE PROCEDURE dbo.usp_MigratePayerService_BillerId
> @.CREDID BIGINT,
> @.BILLERID BIGINT
> AS
> DECLARE @.errnum INT
> SET NOCOUNT ON
> ----
> UPDATE PayerDB.dbo.Payer
> SET BillerId = @.BILLERID
> FROM Migration.dbo.PayerXref X
> JOIN PayerDB.dbo.Payer P
> ON(X.AccountUserID = P.BillerId)
> WHERE X.BillerId = @.BILLERID
> ----
> SET @.errnum = @.@.ERROR
> IF @.errnum<>0
> BEGIN
> GOTO BAILOUT
> END
> ----
> UPDATE PayerDB.dbo.Account
> SET BillerId = @.BILLERID
> FROM Migration.dbo.AccountXref X
> JOIN PayerDB.dbo.Account A
> ON(X.AccountID_TP3 = A.BillerId)
> WHERE x.BillerId = @.BILLERID
> ----
> SET @.errnum = @.@.ERROR
> ----
> BAILOUT:
> RETURN @.errnum
> --End of SP
> --Create statements for tables
> CREATE TABLE dbo.Payer
> (PayerId bigint IDENTITY (1, 1) NOT NULL ,
> Identifier varchar (50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
> BillerId bigint NOT NULL)
> --other fields removed for sake of space
> CREATE TABLE dbo.PayerXref
> (AccountUserID bigint NOT NULL ,
> PayerId bigint NULL ,
> BillerId bigint NOT NULL ,
> Migrated bit NOT NULL CONSTRAINT DF_PayerXref_Migrated DEFAULT (0),
> ErrorDescription varchar (4000) COLLATE Latin1_General_CI_AS NULL)
>|||Thanks for the input Dave.
Please explain how yours is different than mine... other than less typing?
Kevin
"Dave Markle" <"dma[remove_ZZ]ZZrkle" wrote:

> instead of:
> UPDATE PayerDB.dbo.Payer
> SET BillerId = @.BILLERID
> FROM Migration.dbo.PayerXref X
> JOIN PayerDB.dbo.Payer P
> ON(X.AccountUserID = P.BillerId)
> WHERE X.BillerId = @.BILLERID
> use the alias, P:
> UPDATE P
> SET BillerId = @.BILLERID
> FROM Migration.dbo.PayerXref X
> JOIN PayerDB.dbo.Payer P
> ON(X.AccountUserID = P.BillerId)
> WHERE X.BillerId = @.BILLERID
>
> kevin wrote:
>|||Sorry, it was ambiguous to me, but it after running some tests to
convince myself, it wasn't ambiguous to SQL Server.
Anyway, what's your data look like? Can you post the DDL of those two
tables? I don't see a problem with your T-SQL, per se.
-Dave
kevin wrote:
> Thanks for the input Dave.
> Please explain how yours is different than mine... other than less typing?
> Kevin
> "Dave Markle" <"dma[remove_ZZ]ZZrkle" wrote:
>|||You do know that the proprietrary UPDATE.. FROM syntax is ambigous and
not portable? First it r makes no sense in terms of the SQL language
model. A FROM clause is always suppose effectively materialize a
working table that disappears at the end of the statement. Likewise,
an alias is supposed to act as it materializes a new working table with
the data from the original table expression in it. To be consistent,
this syntax says that you have done nothing to the base table.
Sybase and some other vendors had the same syntax but with different
semantics. Worst of both worlds!
And on top of that, it is unpredictable. This is a simple example from
Adam Machanic
CREATE TABLE Foo
(col_a CHAR(1) NOT NULL,
col_b INTEGER NOT NULL);
INSERT INTO Foo VALUES ('A', 0);
INSERT INTO Foo VALUES ('B', 0);
INSERT INTO Foo VALUES ('C', 0);
CREATE TABLE Bar
(col_a CHAR(1) NOT NULL,
col_b INTEGER NOT NULL);
INSERT INTO Bar VALUES ('A', 1);
INSERT INTO Bar VALUES ('A', 2);
INSERT INTO Bar VALUES ('B', 1);
INSERT INTO Bar VALUES ('C', 1);
You run this proprietary UPDATE with a FROM clause:
UPDATE Foo
SET Foo.col_b = Bar.col_b
FROM Foo INNER JOIN Bar
ON Foo.col_a = Bar.col_a;
The result of the update cannot be determined. The value of the column
will depend upon either order of insertion, (if there are no clustered
indexes present), or on order of clustering (but only if the cluster
isn't fragmented).
What you wanted was more like this:
UPDATE Payers -- more than one?
SET biller_id = @.my_biller_id
WHERE EXISTS
(SELECT *
FROM Migration.bo.PayerXref AS X
WHERE X.account_user_id = Payers.biller_id);
UPDATE Accounts
SET biller_id = @.my_biller_id
WHERE EXISTS
(SELECT *
FROM Migration.bo.PayerXref AS X
WHERE X.account_user_id_tp3 = Payers.biller_id);
How many different names does the biller_id data element have/ At
least three names for the *same* data element, which is proof that the
design is a screwed up mess.
The DDL you did post made no sense. No keys, names like "identifier"
that is huge (the only real code I know that is that long is the IBAN),
you have biller_id in a Payers table, NULLs where they should not be,
etfc.|||--CELKO--,
Thanks for your input.

> How many different names does the biller_id data element have/ At
> least three names for the *same* data element, which is proof that the
> design is a screwed up mess.
Not sure what you exactly mean by "different names". The biller is our
customer and the payer is the biller's customer. its a one to many
relationship. Payers may have many Accounts. One-to-Many again. If you ar
e
referring to the fact that the biller_id is on both tables, I agree with you
(see #1 below).

> The DDL you did post made no sense. No keys, names like "identifier"
> that is huge (the only real code I know that is that long is the IBAN),
> you have biller_id in a Payers table, NULLs where they should not be,
> etfc.
See #3 below
1. I am simply the contractor migrating data from an old db schema to a new
one. I did not design the layout or select the object names so I have no
need to defend or explain them. Even so, what is so reprehensible about
"Identifier" as a field name and how do you "know" that there are "...NULLs
where they should not be"? Shouldn't they be where your business needs say
they should be?
2. About proprietary syntax: I am using SQL Server and not SYBASE or SAS or
MYSQL etc. This is a SQL Server forum, not an ANSI SQL forum. My employer
is 100% Microsoft pimped and a damn good Ho at that. A Ho would be a fool
not to ride in the Pimp's caddy. She paid for it!
3.The Payer table does have a primary key on the IDENTITY field. I assumed
that that was a given... but there is a saying about assuming.
CREATE TABLE [dbo].[Payer] (
[PayerId] [bigint] IDENTITY (1, 1) NOT NULL ,
[Identifier] [varchar] (50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[BillerId] [bigint] NOT NULL ,
[FirstName] [varchar] (30) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[LastName] [varchar] (30) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[EmailAddress] [varchar] (100) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[Street1] [varchar] (50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[Street2] [varchar] (50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[City] [varchar] (50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[StateCode] [varchar] (2) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[CountryCode] [varchar] (10) COLLATE SQL_Latin1_General_CP1_CI_AS NULL
CONSTRAINT [DF_Payer_CountryCode] DEFAULT ('USA'),
[PostalCode] [varchar] (9) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[PasswordHash] [varchar] (100) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[PIN] [varchar] (20) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[Phone] [varchar] (12) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[PersistPaymentInfo] [bit] NOT NULL CONSTRAINT
[DF_Payer_PersistPaymentInfo] DEFAULT (1),
[ExpirationDate] [datetime] NULL ,
[Created] [datetime] NOT NULL CONSTRAINT [DF_Payer_Created] DEFAULT
(getdate()),
[CreatedBy] [varchar] (100) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL
CONSTRAINT [DF_Payer_CreatedBy] DEFAULT ('System'),
[Modified] [datetime] NULL ,
[ModifiedBy] [varchar] (100) COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
[rowguid] uniqueidentifier ROWGUIDCOL NOT NULL CONSTRAINT
[DF__Payer__rowguid__3D7E1B63] DEFAULT (newid()),
CONSTRAINT [PK_payer_1] PRIMARY KEY CLUSTERED
(
[PayerId]
) ON [PRIMARY]
) ON [PRIMARY]
GO

No comments:

Post a Comment