[SQL] UPDATE stored procedure met conditionele kolommen

[Christiaan]

Ik heb een stored procedure die een normale update doet:

UPDATE [tabel]
SET kolom1 = @kolom1,
kolom2 = @kolom2
WHERE identity = @identity

Deze query heb ik in een stored procedure geplaatst die @identity, @kolom1 en @kolom2 meekrijgt als parameters. Tot zover werkt het. Wat ik echter wil is de mogelijkheid om bijvoorbeeld @kolom2 niet te specificeren, waardoor deze ook niet wordt geupdate. Het systeem waar deze procedures voor zijn maakt het voor gebruikers namelijk mogelijk velden te 'bevriezen', waardoor ze ze niet langer kunnen editten. De UPDATE hoeft dus alleen niet-bevroren kolommen te updaten. Hoe kan ik dat makkelijk doen? Ik wil liever geen dynamische SQL gebruiken (execute_sp), omdat dat kwetsbaarder is voor SQL-injectie.

[gorgi_19]

Christiaan schreef op donderdag 25 mei 2006 @ 09:58:
Ik heb een stored procedure die een normale update doet:

UPDATE [tabel]
SET kolom1 = @kolom1,
kolom2 = @kolom2
WHERE identity = @identity

Deze query heb ik in een stored procedure geplaatst die @identity, @kolom1 en @kolom2 meekrijgt als parameters. Tot zover werkt het. Wat ik echter wil is de mogelijkheid om bijvoorbeeld @kolom2 niet te specificeren, waardoor deze ook niet wordt geupdate. Het systeem waar deze procedures voor zijn maakt het voor gebruikers namelijk mogelijk velden te 'bevriezen', waardoor ze ze niet langer kunnen editten. De UPDATE hoeft dus alleen niet-bevroren kolommen te updaten. Hoe kan ik dat makkelijk doen? Ik wil liever geen dynamische SQL gebruiken (execute_sp), omdat dat kwetsbaarder is voor SQL-injectie.

En is het geen optie om een parametrized query dynamisch op te bouwen vanuit je applicatie?

[Christiaan]

gorgi_19 schreef op donderdag 25 mei 2006 @ 10:01:
En is het geen optie om een parametrized query dynamisch op te bouwen vanuit je applicatie?

Ik kwam je betoog tegen SPs elders op het forum tegen toen ik aan het zoeken was naar een oplossing Maar het antwoord is nee; ik wil al mijn queries in stored procedures houden (anders staat er SQL-code verdeeld over meerdere plaatsen). Wat ik eigenlijk hoop is dat er een waarde is die voor @kolom2 mee kan geven, waardoor SQL-Server weet dat die kolom niet geupdate hoeft te worden (je kunt @kolom2 helaas niet leeg laten).

[ Voor 6% gewijzigd door Christiaan op 25-05-2006 10:03 ]

[gorgi_19]

Christiaan schreef op donderdag 25 mei 2006 @ 10:03:
Ik kwam je betoog tegen SPs elders op het forum tegen toen ik aan het zoeken was naar een oplossing Maar het antwoord is nee; ik wil al mijn queries in stored procedures houden (anders staat er SQL-code verdeeld over meerdere plaatsen). Wat ik eigenlijk hoop is dat er een waarde is die voor @kolom2 mee kan geven, waardoor SQL-Server weet dat die kolom niet geupdate hoeft te worden (je kunt @kolom2 helaas niet leeg laten).

Dan nog kan je ze centraliseren dmv een DAL, maar dat terzijde

Een oplossing wat zou kunnen werken:
Ga met CASE / IF constructies aan de gang, waarbij je controleert of een waarde gesloten is. Indien gesloten is, update dan het veld met z'n eigen waarde, anders update je de nieuwe waarde
Of een variant er op: sla de waarden op in tijdelijke variabelen en vervang de tijdelijke variabelen met de waarden die geupdate moeten worden. Update vervolgens je rij met die tijdelijke variabelen.

Ik verwacht echter wel dat er betere opties, want dit zal wel een penalty (qua performance) met zich meebrengen

[ Voor 10% gewijzigd door gorgi_19 op 25-05-2006 10:08 ]

[whoami]

Waarom zou sp_executesql je kwetsbaar maken voor SQL injection ? Met die sp_executesql kan je ook parameters gebruiken; kijk maar eens in de Books Online; de eerste parameter is je SQL statement, de 2de parameter is de definitie van de parameters, en de 3de param. bevat de waardes voor de parameters.
Kijk ook eens wat de profiler doet als je een parametrized query uitvoert, volgens mij zal je zien dat er ook gebruik gemaakt wordt van sp_executesql.

[Christiaan]

whoami schreef op donderdag 25 mei 2006 @ 11:37:
Waarom zou sp_executesql je kwetsbaar maken voor SQL injection ? Met die sp_executesql kan je ook parameters gebruiken; kijk maar eens in de Books Online; de eerste parameter is je SQL statement, de 2de parameter is de definitie van de parameters, en de 3de param. bevat de waardes voor de parameters.
Kijk ook eens wat de profiler doet als je een parametrized query uitvoert, volgens mij zal je zien dat er ook gebruik gemaakt wordt van sp_executesql.

Ik heb dat iig net even uit zitten zoeken, en je hebt helemaal gelijk. Dit was ook de oplossing. In mijn hoofd had ik nog ergens het idee dat het een variabelen in de string werden geplakt, ala 'UPDATE [tabel] SET [kolom1] = ' + @kolom1 + ', ' enz. Maar dat hoeft natuurlijk helemaal niet, aangezien je ook parameters mee kan geven. SQL Injectie is dan dus volgens mij helemaal niet mogelijk.

Dit is nu de oplossing geworden:

code:

CREATE PROCEDURE QD_ProfielenDetailsAlgemeen_Update ( @intRecordID BIGINT, @intGeboortedag INT, @intGeboortemaand INT, @intGeboortejaar INT, @intGeboortelandID INT, @intGeboorteprovincieID INT, @intGeboorteplaatsID INT, @intBurgerlijkestaatID INT, @intGewensterelatieID INT, @intKinderen INT, @intKinderenAantal INT, @strKinderenleeftijd VARCHAR(55), @intKinderenthuisaantal TINYINT, @intKinderwens TINYINT, @intModifierID INT)
AS
   DECLARE @strQuery NVARCHAR(4000)
   DECLARE @strParams NVARCHAR(500)

   SET @strParams = N'@intRecordID BIGINT, @intGeboortedag INT, @intGeboortemaand INT, @intGeboortejaar INT, @intGeboortelandID INT, @intGeboorteprovincieID INT, @intGeboorteplaatsID INT, @intBurgerlijkestaatID INT, @intGewensterelatieID INT, @intKinderen INT, @intKinderenaantal VARCHAR(55), @strKinderenleeftijd VARCHAR(55), @intKinderenthuisaantal TINYINT, @intKinderwens TINYINT, @intModifierID INT'
   SET @strQuery = N'UPDATE profielen_details_algemeen
   SET modifieddate = GetDate(),'
   IF NOT @intGeboorteDag = -1 SET @strQuery = @strQuery + N'geboortedag = @intGeboortedag,'
   IF NOT @intGeboorteMaand = -1 SET @strQuery = @strQuery + N'geboortemaand = @intGeboortemaand,'
   IF NOT @intGeboorteJaar = -1 SET @strQuery = @strQuery + N'geboortejaar = @intGeboortejaar,'
   IF NOT @intGeboorteLandID = -1 SET @strQuery = @strQuery + N'geboortelandID = @intGeboortelandID,'
   IF NOT @intGeboorteProvincieID = -1 SET @strQuery = @strQuery + N'geboorteprovincieID = @intGeboorteprovincieID,'
   IF NOT @intGeboortePlaatsID = -1 SET @strQuery = @strQuery + N'geboorteplaatsID = @intGeboorteplaatsID,'
   IF NOT @intBurgerlijkeStaatID = -1 SET @strQuery = @strQuery + N'burgerlijkestaatID = @intBurgerlijkestaatID,'
   IF NOT @intGewensteRelatieID = -1 SET @strQuery = @strQuery + N'gewensterelatieID = @intGewensterelatieID,'
   IF NOT @intKinderen = -1 SET @strQuery = @strQuery + N'kinderen = @intKinderen,'
   IF NOT @intKinderenAantal = -1 SET @strQuery = @strQuery + N'kinderenaantal = @intKinderenaantal,'
   IF NOT @strKinderenLeeftijd = '-1' SET @strQuery = @strQuery + N'kinderenleeftijd = @strKinderenleeftijd,'
   IF NOT @intKinderenThuisAantal = -1 SET @strQuery = @strQuery + N'kinderenthuisaantal = @intKinderenthuisaantal,'
   IF NOT @intKinderWens = -1 SET @strQuery = @strQuery + N'kinderwens = @intKinderwens,'
   SET @strQuery = @strQuery + N'modifierID = @intModifierID
   WHERE profielen_details_algemeen.profielID = @intRecordID'
 
   EXECUTE sp_executesql @strQuery, @strParams, @intRecordID, @intGeboortedag, @intGeboortemaand, @intGeboortejaar, @intGeboortelandID, @intGeboorteprovincieID, @intGeboorteplaatsID, @intBurgerlijkestaatID, @intGewensterelatieID, @intKinderen, @intKinderenaantal, @strKinderenleeftijd, @intKinderenthuisaantal, @intKinderwens, @intModifierID
GO

[ Voor 65% gewijzigd door Christiaan op 25-05-2006 13:22 ]

[kenneth]

Christiaan schreef op donderdag 25 mei 2006 @ 10:03:
Ik kwam je betoog tegen SPs elders op het forum tegen toen ik aan het zoeken was naar een oplossing Maar het antwoord is nee; ik wil al mijn queries in stored procedures houden (anders staat er SQL-code verdeeld over meerdere plaatsen).

En het aanroepen van een stored procedure valt niet onder SQL-code?

[UniCache2]

Op de manier waarop je deze string opbouwd is er volgens mij nu juist WEL weer SQL injection mogelijk...

Christiaan schreef op donderdag 25 mei 2006 @ 13:20:
[...]


Ik heb dat iig net even uit zitten zoeken, en je hebt helemaal gelijk. Dit was ook de oplossing. In mijn hoofd had ik nog ergens het idee dat het een variabelen in de string werden geplakt, ala 'UPDATE [tabel] SET [kolom1] = ' + @kolom1 + ', ' enz. Maar dat hoeft natuurlijk helemaal niet, aangezien je ook parameters mee kan geven. SQL Injectie is dan dus volgens mij helemaal niet mogelijk.

Dit is nu de oplossing geworden:

code:

CREATE PROCEDURE QD_ProfielenDetailsAlgemeen_Update ( @intRecordID BIGINT, @intGeboortedag INT, @intGeboortemaand INT, @intGeboortejaar INT, @intGeboortelandID INT, @intGeboorteprovincieID INT, @intGeboorteplaatsID INT, @intBurgerlijkestaatID INT, @intGewensterelatieID INT, @intKinderen INT, @intKinderenAantal INT, @strKinderenleeftijd VARCHAR(55), @intKinderenthuisaantal TINYINT, @intKinderwens TINYINT, @intModifierID INT)
AS
   DECLARE @strQuery NVARCHAR(4000)
   DECLARE @strParams NVARCHAR(500)

   SET @strParams = N'@intRecordID BIGINT, @intGeboortedag INT, @intGeboortemaand INT, @intGeboortejaar INT, @intGeboortelandID INT, @intGeboorteprovincieID INT, @intGeboorteplaatsID INT, @intBurgerlijkestaatID INT, @intGewensterelatieID INT, @intKinderen INT, @intKinderenaantal VARCHAR(55), @strKinderenleeftijd VARCHAR(55), @intKinderenthuisaantal TINYINT, @intKinderwens TINYINT, @intModifierID INT'
   SET @strQuery = N'UPDATE profielen_details_algemeen
   SET modifieddate = GetDate(),'
   IF NOT @intGeboorteDag = -1 SET @strQuery = @strQuery + N'geboortedag = @intGeboortedag,'
   IF NOT @intGeboorteMaand = -1 SET @strQuery = @strQuery + N'geboortemaand = @intGeboortemaand,'
   IF NOT @intGeboorteJaar = -1 SET @strQuery = @strQuery + N'geboortejaar = @intGeboortejaar,'
   IF NOT @intGeboorteLandID = -1 SET @strQuery = @strQuery + N'geboortelandID = @intGeboortelandID,'
   IF NOT @intGeboorteProvincieID = -1 SET @strQuery = @strQuery + N'geboorteprovincieID = @intGeboorteprovincieID,'
   IF NOT @intGeboortePlaatsID = -1 SET @strQuery = @strQuery + N'geboorteplaatsID = @intGeboorteplaatsID,'
   IF NOT @intBurgerlijkeStaatID = -1 SET @strQuery = @strQuery + N'burgerlijkestaatID = @intBurgerlijkestaatID,'
   IF NOT @intGewensteRelatieID = -1 SET @strQuery = @strQuery + N'gewensterelatieID = @intGewensterelatieID,'
   IF NOT @intKinderen = -1 SET @strQuery = @strQuery + N'kinderen = @intKinderen,'
   IF NOT @intKinderenAantal = -1 SET @strQuery = @strQuery + N'kinderenaantal = @intKinderenaantal,'
   IF NOT @strKinderenLeeftijd = '-1' SET @strQuery = @strQuery + N'kinderenleeftijd = @strKinderenleeftijd,'
   IF NOT @intKinderenThuisAantal = -1 SET @strQuery = @strQuery + N'kinderenthuisaantal = @intKinderenthuisaantal,'
   IF NOT @intKinderWens = -1 SET @strQuery = @strQuery + N'kinderwens = @intKinderwens,'
   SET @strQuery = @strQuery + N'modifierID = @intModifierID
   WHERE profielen_details_algemeen.profielID = @intRecordID'
 
   EXECUTE sp_executesql @strQuery, @strParams, @intRecordID, @intGeboortedag, @intGeboortemaand, @intGeboortejaar, @intGeboortelandID, @intGeboorteprovincieID, @intGeboorteplaatsID, @intBurgerlijkestaatID, @intGewensterelatieID, @intKinderen, @intKinderenaantal, @strKinderenleeftijd, @intKinderenthuisaantal, @intKinderwens, @intModifierID
GO

[_Thanatos_]

Je kan de update toch in tweeën splitsen? Als veld A geüpdate moet worden, dan doe je dat. En als veld B geüpdate moet worden, dan doe je die erna. Als je 12 velden hebt, zijn het weliswaar 12 update statements, maar het werkt wel.

Andere oplossing is om de bestaande data eerst op te halen en het in variabelen te zetten. Dan een voor een die variabelen updaten waar nodig, en dan 1 dik vet update-statement eronder.

[Christiaan]

_Thanatos_ schreef op donderdag 25 mei 2006 @ 17:29:
Je kan de update toch in tweeën splitsen? Als veld A geüpdate moet worden, dan doe je dat. En als veld B geüpdate moet worden, dan doe je die erna. Als je 12 velden hebt, zijn het weliswaar 12 update statements, maar het werkt wel.

Dat kan wel, maar is wat lastig met tabellen met meer dan 30 kolommen (waar het in dit geval wel om gaat). 30 update statements is nogal traag, zeker als er veel mensen zijn die het systeem tegelijk gebruiken voor updates. Ik denk ook dat locking beter verloopt als alles in 1 query zit dan als het over 30 verspreid is.

Andere oplossing is om de bestaande data eerst op te halen en het in variabelen te zetten. Dan een voor een die variabelen updaten waar nodig, en dan 1 dik vet update-statement eronder.

Dat vind ik niet een bijzonder elegante oplossing. Bovendien is het meer werk als er kolommen bij komen. Dan moet je ook code aanpassen waar eerst data wordt opgehaald.

[Christiaan]

UniCache2 schreef op donderdag 25 mei 2006 @ 14:24:
Op de manier waarop je deze string opbouwd is er volgens mij nu juist WEL weer SQL injection mogelijk...

Ik krijg het in elk geval niet voor elkaar Hoe zou het volgens jou mogelijk zijn? Als je een van de parameters voorziet van een SQL-statement, zoals 'DROP TABLE', dan wordt dat gewoon als string behandeld.

[ Voor 18% gewijzigd door Christiaan op 25-05-2006 18:02 ]

[Christiaan]

kenneth schreef op donderdag 25 mei 2006 @ 14:21:
En het aanroepen van een stored procedure valt niet onder SQL-code?

Nee. De queries staan in de database, niet in de code. Alleen de aanroep staat in de code. Dat lijkt me toch wel duidelijk anders. Simpel voorbeeld is het gebruik van 1 SQL-query op meerdere plekken in je code. Je roept simpelweg dezelfde SP aan. Als je je SQL-code direct in de code op zou nemen, moet je het dubbel opnemen.

Bovendien vind ik het plezierig dat ik snel mijn queries kan vinden. Als ik tabellen aanpas, hoef ik alleen maar de benodigde SPs aan te passen (die netjes onder elkaar vermeld staan in de lijst in SQL Server). Als ik de SQL-query in mijn code zelf zou hebben staan, moet ik dat overal op gaan zoeken. Niet erg handig.

[ Voor 25% gewijzigd door Christiaan op 25-05-2006 18:01 ]

[kenneth]

Ik heb anders gewoon per tabel één klasse die ik aanspreek, en daar zit de SQL-code in.

En je tweede argument klopt niet, want je moet ook de code die de SP aanroept aanpassen

[Verwijderd]

Ik heb wel eens de volgende constructie gebruikt (voorbeeld):

code:

CREATE PROCEDURE uspUpdateCompany
(
    @CompanyId int,
    @Name varchar(50) = null,
    @Address varchar(50) = null,
    @Postcode char(6) = null,
    @Place varchar(50) = null,
    @Telephone varchar(20) = null
)
AS
BEGIN
        UPDATE
                tblCompany
        SET 
                [Name] = ISNULL(@Name, [Name]),
                Address = ISNULL(@Address, Address),
                Postcode = ISNULL(@Postcode, Postcode),
                Place = ISNULL(@Place, Place),
                Telephone = ISNULL(@Telephone, Telephone)
        WHERE 
                CompanyId = @CompanyId
END
GO

De parameters die je niet wilt updaten, moet je dan niet toevoegen vanuit de code. Hierdoor krijgen deze de default waarde (null). En door de ISNULL() functie krijgen de velden alleen maar een nieuwe waarde als de parameter niet null is.

[_Thanatos_]

Christiaan schreef op donderdag 25 mei 2006 @ 17:58:
Dat vind ik niet een bijzonder elegante oplossing. Bovendien is het meer werk als er kolommen bij komen. Dan moet je ook code aanpassen waar eerst data wordt opgehaald.

Als je er kolommen bij verzint, moet je je SP toch aanpassen, en het update-statement ook. Dus wat is het extra werk dan? Enne, hoe vaak verandert je DB-model nou? Ik hoop bijna nooit?

[Verwijderd]

Verwijderd schreef op donderdag 25 mei 2006 @ 18:48:
Ik heb wel eens de volgende constructie gebruikt (voorbeeld):

De parameters die je niet wilt updaten, moet je dan niet toevoegen vanuit de code. Hierdoor krijgen deze de default waarde (null). En door de ISNULL() functie krijgen de velden alleen maar een nieuwe waarde als de parameter niet null is.

Ik heb dit ook een keertje gedaan voor een SELECT.
Werkt op zich goed, maar met een UPDATE heb je wel een probleem als een veld van waarde 'iets' naar NULL moet wijzigen... dat lukt niet :-)

Ennuh voor _Thanatos_, een datamodel veranderd natuurlijk altijd! Daar moet je altijd rekening mee houden... Maar je oplossing is misschien niet elegant, maar wel correct.

[__fred__]

Verwijderd schreef op vrijdag 26 mei 2006 @ 09:19:
[...]

Ik heb dit ook een keertje gedaan voor een SELECT.
Werkt op zich goed, maar met een UPDATE heb je wel een probleem als een veld van waarde 'iets' naar NULL moet wijzigen... dat lukt niet :-)

En daar kun je weer een extra bit parameter voor meegeven per kolom.

[Verwijderd]

_Thanatos_ schreef op donderdag 25 mei 2006 @ 17:29:
Je kan de update toch in tweeën splitsen? Als veld A geüpdate moet worden, dan doe je dat. En als veld B geüpdate moet worden, dan doe je die erna. Als je 12 velden hebt, zijn het weliswaar 12 update statements, maar het werkt wel.

Daar maak je bij je klanten vrienden mee, zeker wanneer 't om een tabel met 200+ velden gaat. Stored procs kunnen goed voor wat extra performance zorgen, maar op deze manier leg je de boel helemaal lam.

Ik denk overigens dat je inserts, updates, etc. niet in stored procs vast moet leggen, maar in je DAL. Maar dat zal wel tegen het zere been zijn van een hoop "old school" database proggers...

[_Thanatos_]

Een tabel met 200+ velden
Dat smaakt gelijk weer zo zuur. Of het dan gaat om SP's of om je client-code, onderhoud van zoiets lijkt me even weinig vrienden mee te maken. Voor zover klant het iets boeit hoe de applicatie van binnen werkt

En ach, het is een hele discussie op zich of je updates/inserts in de database of in de DAL moet vastleggen... Ik denk dat dat een beetje offtopic zou gaan

[kenneth]

Ik vind het wel geinig ... koste wat kost met zoveel mogelijk behoud van alle flexibiliteit, tegenargumenten negerend tóch SP's willen gebruiken