[SQL] Compleet statement in variable

[martijn946]

Beste Tweakers,

Iedereen heeft recht op zijn dagelijkse n00b-momentje. Zo heb ik er een vandaag.

Dagelijks loopt er een scriptje op onze database die gebruikers na 18 maanden inactiviteit blokkeert. Er worden 2 tabellen bijgewerkt (geanonimiseerd) na een select van de inactieve users.

Het gaat mij om het "SELECT DISTINCT" gedeelte. Dit komt 2 keer voor en is identiek. Elke aanpassing moet ik dus 2 keer doen, 1 in de bovenste en 1 in de onderste. Dit moet simpeler kunnen dacht ik zo. De gehele select in een variable. Dus met een hoop DECLARE, SET en een partij komma's, quote's etc etc in de weer geweest en ... 0 resultaat: MSMS vreet het niet.

Hoe vlieg ik dit aan??

SQL: query.txt

------------------------------
-- This script is used to block and anonimyze useraccounts after 18 months of inactivity (GDPR).
-- The field [LastActiveDate] is used for this and this field is active since 04-03-2020.
-- Therefore, the script itself is active sinds 04-03-2020 + 18 months = 04-09-2021.
-- First, it cleans the UserRevision table with the inactive users.
-- Afterwards the useraccount itself is being blocked.
--
-- Be aware that all admin accounts and accounts from TEST countries are ignored.
--
------------------------------

UPDATE [DP].[dbo].[UserRevisions]
   SET [Name] = 'Anonymized'
      ,[Activated] = 0
      ,[EmailAddress] = CAST([UserID] as varchar) + '@Anonymized.com'
      ,[CompanyName] = 'Anonymized'
      ,[CompanyAddress] = 'Anonymized'
      ,[CompanyZipCode] = 'Anonymized'
      ,[CompanyCity] = 'Anonymized'
      ,[CompanyCountry] = NULL
      ,[CompanyPhoneNumber] = 0
      ,[CompanyEmailAddress] = 'Anonymized'
      ,[] = 0
      ,[PhoneNumber] = 0
      ,[JobFunction] = 'Anonymized'
      ,[LastModifiedByUserId] = 30480
      ,[WorkflowState] = 'Deactivated'
    WHERE [UserID] IN 
( 
SELECT DISTINCT U.[UserId] FROM [DP].[dbo].[Users] U

INNER JOIN [DP].[dbo].[TsCountries] C
ON U.[TsCountryId] = C.[TsCountryId]

INNER JOIN [dbo].[UserAccountGroups] AG
ON U.[UserId] = AG.[UserId]

WHERE   
        U.[WorkflowState] ='Active' AND         -- All active users in state
        U.[Activated] = 1 AND                   -- All active users in enabled state
        U.[Name] NOT LIKE '%admin%' AND         -- Excluding names with "Admin" in it
        AG.AccountGroupId <> 32 AND             -- No Account Group "Administrators"
        AG.AccountGroupId <> 57 AND             -- No Account Group "Klantnaam Specificiation"
        AG.AccountGroupId <> 52 AND             -- No Account Group "Klantnaam Support"
        AG.AccountGroupId <> 34 AND             -- No Account Group "User Administrator"
        U.[LastActiveDate] < DATEADD(MONTH,-18,GETDATE()) AND
        C.[TsCountryId] <> 124 AND
        C.[TestCountry] = 0
)


UPDATE [DP].[dbo].[Users]
      SET 
       [Name] = 'Anonymized',
       [Activated] = 0,
       [LoginName] = NULL,
       [EmailAddress] = CAST([UserID] as varchar) + '@Anonymized.com',
       [CompanyName] = NULL,
       [CompanyAddress] = NULL,
       [CompanyZipCode] = NULL,
       [CompanyCity] = NULL,
       [CompanyCountry] = NULL,
       [CompanyPhoneNumber] = NULL,
       [CompanyEmailAddress] = 'Anonymized',
       [OldEmailAddress] = CAST([UserID] as varchar) + '@Anonymized.com',
       [] = NULL,
       [PhoneNumber] = NULL,
       [JobFunction] = NULL,
       [UserRoleId] = 1,
       [LastModifiedByUserId] = 30480,
       [SecurityLastModified] = GETDATE(),
       [WorkflowState] = 'Deactivated',
       [MobilePhoneNumber] = 1
   WHERE [UserID] IN 

(
SELECT DISTINCT U.[UserId] FROM [DP].[dbo].[Users] U

INNER JOIN [DP].[dbo].[TsCountries] C
ON U.[TsCountryId] = C.[TsCountryId]

INNER JOIN [dbo].[UserAccountGroups] AG
ON U.[UserId] = AG.[UserId]

WHERE   
        U.[WorkflowState] ='Active' AND         -- All active users in state
        U.[Activated] = 1 AND                   -- All active users in enabled state
        U.[Name] NOT LIKE '%admin%' AND         -- Excluding names with "Admin" in it
        AG.AccountGroupId <> 32 AND             -- No Account Group "Administrators"
        AG.AccountGroupId <> 57 AND             -- No Account Group "Klantnaam Specificiation"
        AG.AccountGroupId <> 52 AND             -- No Account Group "Klantnaam Support"
        AG.AccountGroupId <> 34 AND             -- No Account Group "User Administrator"
        U.[LastActiveDate] < DATEADD(MONTH,-18,GETDATE()) AND
        C.[TsCountryId] <> 124 AND
        C.[TestCountry] = 0
)

[Tsurany]

Ik zou de revision tabel via een trigger updaten, dan hoef je de history niet in elke operatie op de Users tabel mee te nemen maar gebeurt het automatisch bij elke update ongeacht hoe de update gedaan wordt.

[KabouterSuper]

Ik zou het SELECT DISTINCT gedeelte ombouwen naar een view (bijv met naam INACTIVE_USERS). Vervolgens kan je je twee update statements ombouwen naar

code:

update <table> set .... where user_id in
 (select user_id from INACTIVE_USERS)

[urk_forever]

Kan je die in-actieve gebruikers niet gewoon in een temp table stoppen en die steeds joinen?

[martijn946]

Donders mensen, ik mag wel zeer zeggen: zeer goede suggesties waar ik bij allen nog niet aan had gedacht. Ik dank jullie hartelijk!

[KabouterSuper]

martijn946 schreef op woensdag 8 september 2021 @ 13:27:
Donders mensen, ik mag wel zeer zeggen: zeer goede suggesties waar ik bij allen nog niet aan had gedacht. Ik dank jullie hartelijk!

Mooi, weer een probleem opgelost. Overigens, mocht ik mezelf ooit onsterfelijk willen maken in jullie database, dan hoef ik mezelf alleen maar kaboutersuperadmin te noemen. Heb je niet een betere conditie dan name not like '%admin%' om admins uit te sluiten?

[GarBaGe]

KabouterSuper schreef op woensdag 8 september 2021 @ 14:14:
[...]

Mooi, weer een probleem opgelost. Overigens, mocht ik mezelf ooit onsterfelijk willen maken in jullie database, dan hoef ik mezelf alleen maar kaboutersuperadmin te noemen. Heb je niet een betere conditie dan name not like '%admin%' om admins uit te sluiten?

en waarom zou een admin account na 18 maanden niet OOK uitgeschakeld moeten worden?
doorgaans hebben mensen maar 5 weken vakantie per jaar, niet 18 maanden...

[CurlyMo]

Volgens mij kan je met MSSQL ook insert en update statements in je CTE verwerken. Je krijgt dan dit:

SQL:

WITH lijst AS (
    SELECT DISTINCT U.[UserId] FROM [DP].[dbo].[Users] U
    INNER JOIN [DP].[dbo].[TsCountries] C
    ON U.[TsCountryId] = C.[TsCountryId]
    INNER JOIN [dbo].[UserAccountGroups] AG
    ON U.[UserId] = AG.[UserId]
    WHERE   
        U.[WorkflowState] ='Active' AND         -- All active users in state
        U.[Activated] = 1 AND                   -- All active users in enabled state
        U.[Name] NOT LIKE '%admin%' AND         -- Excluding names with "Admin" in it
        AG.AccountGroupId <> 32 AND             -- No Account Group "Administrators"
        AG.AccountGroupId <> 57 AND             -- No Account Group "Klantnaam Specificiation"
        AG.AccountGroupId <> 52 AND             -- No Account Group "Klantnaam Support"
        AG.AccountGroupId <> 34 AND             -- No Account Group "User Administrator"
        U.[LastActiveDate] < DATEADD(MONTH,-18,GETDATE()) AND
        C.[TsCountryId] <> 124 AND
        C.[TestCountry] = 0
    )
),
tabel1 AS (
    UPDATE [DP].[dbo].[UserRevisions]
    OUTPUT INSERTED.UserID
    SET [Name] = 'Anonymized'
        ,[Activated] = 0
        ,[EmailAddress] = CAST([UserID] as varchar) + '@Anonymized.com'
        ,[CompanyName] = 'Anonymized'
        ,[CompanyAddress] = 'Anonymized'
        ,[CompanyZipCode] = 'Anonymized'
        ,[CompanyCity] = 'Anonymized'
        ,[CompanyCountry] = NULL
        ,[CompanyPhoneNumber] = 0
        ,[CompanyEmailAddress] = 'Anonymized'
        ,[] = 0
        ,[PhoneNumber] = 0
        ,[JobFunction] = 'Anonymized'
        ,[LastModifiedByUserId] = 30480
        ,[WorkflowState] = 'Deactivated'
    WHERE [UserID] IN (SELECT [UserId] FROM lijst)
)
UPDATE [DP].[dbo].[Users]
OUTPUT INSERTED.UserID
SET 
     [Name] = 'Anonymized',
     [Activated] = 0,
     [LoginName] = NULL,
     [EmailAddress] = CAST([UserID] as varchar) + '@Anonymized.com',
     [CompanyName] = NULL,
     [CompanyAddress] = NULL,
     [CompanyZipCode] = NULL,
     [CompanyCity] = NULL,
     [CompanyCountry] = NULL,
     [CompanyPhoneNumber] = NULL,
     [CompanyEmailAddress] = 'Anonymized',
     [OldEmailAddress] = CAST([UserID] as varchar) + '@Anonymized.com',
     [] = NULL,
     [PhoneNumber] = NULL,
     [JobFunction] = NULL,
     [UserRoleId] = 1,
     [LastModifiedByUserId] = 30480,
     [SecurityLastModified] = GETDATE(),
     [WorkflowState] = 'Deactivated',
     [MobilePhoneNumber] = 1
WHERE [UserID] IN (SELECT [UserId] FROM lijst)
UNION ALL
SELECT * FROM tabel1

Die OUTPUT INSERTED.UserID heb ik net even van internet geplukt. Het gaat er daarbij om dat je de tweede CTE wel moet aanroepen, omdat die anders wordt overgeslagen. Ik dacht dat op te lossen door gewoon een lijstje van updated UserID's te laten teruggeven. Kan zijn dat dat niet zo werkt, maar het idee is hetzelfde. Ik ben zelf vooral een PostgreSQL dialect spreker en geen MSSQL

Ik zeg niet dat het een beter idee is, maar dat het óók een optie ter overweging is.

[martijn946]

KabouterSuper schreef op woensdag 8 september 2021 @ 14:14:
[...]
Heb je niet een betere conditie dan name not like '%admin%' om admins uit te sluiten?

Terechte vraag. Laat ik zeggen dat mijn SQL kennis op schaal van 1-10 eeeh....3 is. Dus deze conditie zal niet de schoonheidsprijs verdienen, maar werkt wel. Ik had het ook up UserID kunnen doen of een bepaald land waar de admin aan gekoppeld is (zie de JOIN). Ik vond mijn manier echter in 1 oogopslag duidelijk wat het doet.

GarBaGe schreef op woensdag 8 september 2021 @ 14:24:
en waarom zou een admin account na 18 maanden niet OOK uitgeschakeld moeten worden?
doorgaans hebben mensen maar 5 weken vakantie per jaar, niet 18 maanden...

Dat is een hele andere discussie

In het kader van GDPR en shit hebben we besloten alle gebruikers niet actief zijn op onze database te anonimiseren EN te blokkeren na 18 maanden van inactiviteit. Een adminaccount (die nooit gebruikt mag worden, maar toch belangrijk is in het kader van nood etc) moet uitgesloten blijven. Befehl ist befehl!

CurlyMo schreef op woensdag 8 september 2021 @ 14:25:
Volgens mij kan je met MSSQL ook insert en update statements in je CTE verwerken. Je krijgt dan dit:...
Ik zeg niet dat het een beter idee is, maar dat het óók een optie ter overweging is.

Dit is voor mij hogere SQL-kunde. Ik snap wat het doet als ik het lees, maar zou er zelf nooit op zijn gekomen, soms denk ik te moeilijk.....denk ik


Overigens heb ik mijn "probleem" opgelost met de VIEW optie die werd geboden en liep als de brandweer.

[ Voor 3% gewijzigd door martijn946 op 08-09-2021 15:41 ]

[Woy]

martijn946 schreef op woensdag 8 september 2021 @ 15:27:
[...]
Terechte vraag. Laat ik zeggen dat mijn SQL kennis op schaal van 1-10 eeeh....3 is. Dus deze conditie zal niet de schoonheidsprijs verdienen, maar werkt wel. Ik had het ook up UserID kunnen doen of een bepaald land waar de admin aan gekoppeld is (zie de JOIN). Ik vond mijn manier echter in 1 oogopslag duidelijk wat het doet.

Voeg dan gewoon een veld toe die aangeeft of het een admin account is en niet of er toevallig admin in de naam voorkomt.

[DataGhost]

Woy schreef op woensdag 8 september 2021 @ 16:00:
[...]

Voeg dan gewoon een veld toe die aangeeft of het een admin account is en niet of er toevallig admin in de naam voorkomt.

Inderdaad. Koenraad Minnema zou niet blij zijn met deze regel als blijkt dat over 20 jaar zijn gegevens nog steeds in de database staan.

[CurlyMo]

martijn946 schreef op woensdag 8 september 2021 @ 15:27:
[...]
Dit is voor mij hogere SQL-kunde. Ik snap wat het doet als ik het lees, maar zou er zelf nooit op zijn gekomen, soms denk ik te moeilijk.....denk ik

Een CTE is een soort view maar dan eentje die je adhoc integreert in je query. Daarbij is bij PostgreSQL het nadeel dat het optimalization gap is. Oftewel, de query planner kan er minder goed mee uit de voeten, dan wanneer je de inhoud repeterend in je query zelf zet. Bijv. bij stukjes logica hergebruiken in meerdere joins. Overigens kan dat ook van toepassing zijn op (slecht geschreven) views. Normaal gesproken gebruik je CTE's ook voor het voorkomen van redundantie in je joins.

Maar in een CTE kunnen ook update, delete en insert (UDI) statements gedraaid worden. Voorwaarden is wel dat er enige vorm van data gegeneerd wordt door de CTE. Dus je CTE UDI moeten data teruggeven anders is de CTE niet aan te roepen in een select zoals ik in het voorbeeld laat zien. Als je dat eenmaal weet dan kan je heel gemakkelijk deze UDI logica in één query met meerdere CTE's te stoppen om bijv. een gebruikers ID aan te maken in een andere tabel als de gebruikers info. Daarmee blijven de statements bij elkaar en draait het geheel per definitie als één transactie. Dat kan je ook in je controller doen of middels triggers. Het is maar wat je voorkeur heeft en de eisen zijn van je architectuur.

[martijn946]

DataGhost schreef op woensdag 8 september 2021 @ 16:07:
[...]

Inderdaad. Koenraad Minnema zou niet blij zijn met deze regel als blijkt dat over 20 jaar zijn gegevens nog steeds in de database staan.

Klopt. Echter worden accounts die handmatig geblokkeerd worden (bijvoorbeeld bij uitdiensttreding) ook geanomiseerd. Dus Koenemeloentje is dan alsnog niet meer traceerbaar.

CurlyMo schreef op woensdag 8 september 2021 @ 17:37:
.... architectuur.

Dank voor je heldere uitleg, wederom weer wijzer geworden.

[Woy]

martijn946 schreef op donderdag 9 september 2021 @ 09:38:
[...]
Klopt. Echter worden accounts die handmatig geblokkeerd worden (bijvoorbeeld bij uitdiensttreding) ook geanomiseerd. Dus Koenemeloentje is dan alsnog niet meer traceerbaar.

Aangezien niet alle accounts handmatig geblokkeerd worden ( Anders had deze query niet zoveel nut ), is dat dus niet voldoende. Punt is gewoon dat mensen de substring 'admin' in hun naam kunnen hebben. Als je een uitputtende lijst met admin account namen hebt zou je ze met een IN ( .... ) clause kunnen afdekken, maar veel makkelijker en beter is gewoon een extra boolean veld IsAdmin o.i.d. ( Of checken op lidmaatschap van een groep ofzo ).

[martijn946]

Woy schreef op donderdag 9 september 2021 @ 09:59:
[...]

Aangezien niet alle accounts handmatig geblokkeerd worden ( Anders had deze query niet zoveel nut ), is dat dus niet voldoende. Punt is gewoon dat mensen de substring 'admin' in hun naam kunnen hebben. Als je een uitputtende lijst met admin account namen hebt zou je ze met een IN ( .... ) clause kunnen afdekken, maar veel makkelijker en beter is gewoon een extra boolean veld IsAdmin o.i.d. ( Of checken op lidmaatschap van een groep ofzo ).

Zeker, is ook zo. Gaan we ook alsnog toevoegen in de database.