Posted by teddysnips on 03/20/06 19:21

Hugo Kornelis wrote:
> On 17 Mar 2006 06:24:01 -0800, teddysnips@hotmail.com wrote:
[...]
> >CREATE TABLE [dbo].[tblMyTable] (
> > [fldCSID] uniqueidentifier ROWGUIDCOL NOT NULL ,
> > [fldSubject][ntext] COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
> > [fldDescription] [ntext] COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
> > [fldKBSubject] [ntext] COLLATE SQL_Latin1_General_CP1_CI_AS NULL ,
> > [fldKBDescription] [ntext] COLLATE SQL_Latin1_General_CP1_CI_AS NULL
> >) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
> >GO
>
> You didn't declare any PRIMARY KEY in this database. I think you
> intended to make the column fldCSID a PRIMARY KEY, but you didn't
> declare it as such.

Weird, it IS the PK! An error with the script, or maybe I was a bit
eager on the editing.

> Does the fldSCID column really have to be uniqueidentifier? If you
> choose to use surrogate keys, then IDENTITY should be the regular
> choice; situations that call for uniqueidentifier are very rare.

At one time the database was replicated.

> Apart from the uniqueidentifier column, all your columns accept NULLs.
> Do you really want to accept rows with just NULLs in your database?
> Nullable columns should be the exception, not the rule.

Couldn't agree more - not my DB design!

> Are you sure that all these columns need to be ntext? I can somewhat
> imagine having descriptions of over 4,000 characters - but subjects? I
> think that you should probably define Subject and KBSubject ar nvarchar
> with an appropriate maximum length (hopefully less than 100, but I don;t
> know your business of course). You might also want to rethiink the
> choice of ntext/nvarchar over text/varchar - unless you really need to
> store characters from non-Western alphabets or other characters that are
> only available in unicode, there's no reason to use double the space
> taken.

The ntext come from the Access upsizing wizard. The original designer
simply left the default values (it would have been memo columns in
Access)

> On to the trigger (I removed the empty lines for readability)
>
> >CREATE TRIGGER PrepopulateKBFieldsFromQuery ON dbo.tblMyTable
> >FOR INSERT
> >AS
> >BEGIN
> >
> >IF UPDATE(fldKBSubject)
> > BEGIN
> > UPDATE
> > tblMyTable
> > SET
> > fldSubject = i.fldKBSubject
> > FROM
> > inserted i INNER JOIN
> > tblMyTable ON i.fldCSID = tblMyTable.fldCSID
> > END
> >
> >IF UPDATE (fldKBDescription)
> > BEGIN
> > UPDATE
> > tblMyTable
> > SET
> > fldDescription = i.fldKBDescription
> > FROM
> > inserted i INNER JOIN
> > tblMyTable ON i.fldCSID = tblMyTable.fldCSID
> > END
> >END
>
> In an INSERT trigger, you don't need IF UPDATE(). It only makes sense in
> an UPDATE trigger; for an INSERT, the IF UPDATE() will be true for each
> column in the table.

Yes, I realise that now - thanks.

> There's also no need to use two seperate update statements. You can
> combine these into one and gain some performance.

I tend to be very "belt and braces" with my code.

> But the most important question, I think, is why you want to do this. If
> the KBSubject and KBDescription are always a copy of the Subject and
> Description columns, why have them?

I want to do it because the underlying application is a query system.
Some queries will form part of a Knowledge Base system. The client
wants the (QUERY)Subject and Description columns to be mirrored by the
KBSubject and KBDescription fields, at least initially. Only the KB
versions will be exposed to the customer.

> Anyway, back to your question:
>
> >I need a trigger (well, I don't *need* one, but it would be optimal!)
> >but I can't get it to work because it references ntext fields.
>
> You can't reference ntext columns in the inserted column. But you can
> join to the base table and get the data from there. (Or you could
> convert the trigger to an instead of trigger, in which case the ntext
> data *WILL* be available in the inserted table - but that's not the
> easiest solution in this case).
>
> CREATE TRIGGER PrepopulateKBFieldsFromQuery
> ON dbo.tblMyTable
> FOR INSERT
> AS
> UPDATE MyTable
> SET Subject = KBSubject,
> Description = KBDescription
> WHERE EXISTS
> (SELECT *
> FROM inserted AS i
> WHERE i.CSID = MyTable.CSID)
> go

And that is absolutely spot on! Many thanks.

Edward

[Back to original message]