Feil i en stored procedure - MSSQL

[Manfred]

Har problemer med en stored procedure som gir meg feilmeldingen "Incorrect syntax near the keyword 'END'", og linjen med "amount = CASE vat WHEN 1 THEN Amount / (1 + (VatAmount / 100)) ELSE Amount END," er markert. Jeg har brukt nøyakti den samme spørringen i en annen procedure som funker helt fint. Det eneste forskjellen er IF-saken rundt selectene i bunnen, og litt med hvilke felter jeg har med i #Tmp-tabellen.

 

Noen som har noen innspill til hva som kan være galt her?

 

CREATE PROCEDURE vat_post
     @year CHAR(4),
     @post VARCHAR(10)
AS

Declare @id Int,
       @Amount Decimal,
       @VatAmount Int,
       @Vat Bit

Create Table #Tmp_vat_post(id Int NOT NULL, VatAmount Integer, Amount Decimal(24,2), Vat Bit)

Declare Account_cursor CURSOR FOR
SELECT	account.ID,
 account.VatAmount,
 amount = CASE vat WHEN 1 THEN Amount / (1 + (VatAmount / 100)) ELSE Amount END,
 account.Vat
FROM account
WHERE account.Date BETWEEN @year + '-01-01' AND @year + '-12-31' AND account.Income = 1

Open Account_cursor

FETCH Next FROM Account_cursor INTO @id, @VatAmount, @Amount, @Vat

While @@Fetch_status=0 Begin

   INSERT INTO #Tmp_vat_post VALUES (@id, @VatAmount,@Amount)

   FETCH Next FROM Account_cursor INTO @id, @VatAmount, @Amount, @Vat
End

IF( @post = 'first' ) Begin
   SELECT SUM(Amount) AS Amount FROM #Tmp_vat_post
End
Else If( @post = 'second' ) Begin
   SELECT SUM(Amount) AS Amount FROM #Tmp_vat_post WHERE Vat = 1
End
Else If( @post = 'third' ) Begin
   SELECT VatAmount, SUM(Amount) FROM #Tmp_vat_post WHERE Vat = 1 GROUP BY VatAmount
End

Close Account_cursor
Deallocate Account_cursor

Return
END

[roac]

Feilen ligger nok et helt annet sted, og SQL Server 2005 Express sier meg:

 

Msg 102, Level 15, State 1, Server MMPOKB, Procedure vat_post, Line 46

Incorrect syntax near 'END'.

 

Grunnen til dette er at den ikke har en matchende begin i begynnelsen av prosedyren.

 

Ellers vil jeg bare få kommentere at dette ser ut til å være et klassisk eksempel på et sted bruk av cursor ikke er nødvendig, eller sågar kan frarådes på det sterkeste. Jeg vil anbefale deg å spesifisere nøyaktig hva du ønsker å gjøre, og så løse det uten bruk av cursor og midlertidige tabeller. Det skal fint la seg gjøre, og være mye mer effektivt.

[Manfred]

haha... så det jo nå... skal ikke ha den END'en på slutten

 

Og nei, dette kan ikke løses på en annen måte enn midlertidige tabeller, dessverre. jeg føkka opp arkitekturen på databasen, og trenger nå disse prosedyrene for å gjøre uttrekkene mulige

 

Edit: det er en del av et regnskapssystem, hvor jeg har vært smart nok til å legge inkl. mva. prisene inn i databasen.

Endret 27. januar 2006 av Manfred

[roac]

haha... så det jo nå... skal ikke ha den END'en på slutten

 

Og nei, dette kan ikke løses på en annen måte enn midlertidige tabeller, dessverre. jeg føkka opp arkitekturen på databasen, og trenger nå disse prosedyrene for å gjøre uttrekkene mulige

 

Edit: det er en del av et regnskapssystem, hvor jeg har vært smart nok til å legge inkl. mva. prisene inn i databasen.

5508866[/snapback]

Dette er fult mulig å gjøre på andre måter, f eks ved hjelp av subqueries.

 

create procedure name parm1, param2 as (
 if param1 =...
 begin

 end else if ...
 begin

 end else
 begin

 end
)

 

og for beregninger, kan du f eks i det ene tilfellet se på noe slikt som:

 

select sum(amount - amount*cast(vatamount as decimal)/(100+vatamount)*vat) from test

 

Videre bør du vurdere noe i retning av

 

... where datepart(year, account.Date) = @year

 

og ha @year som en integerverdi ikke char(4).

 

Jeg mener fortsatt at det skal være fult mulig å løse dette uten bruk av cursorer, og vil på det sterkeste anbefale deg å "grave deg ned" i en bok om T-SQL programmering. cursorer er ressurskrevende. Bruk av midlertidige tabeller er ressurskrevende. Dette til sammen vil gjøre at den løsningen du har lagt opp til vil være langt fra optimal.

 

Hvis jeg nå til slutt skal være ytterst kritisk, noe jeg ofte blir beskyldt for å være, så er det en kritisk designfeil å ha momsrate som heltall, da man ikke har noen garanti for at desimaltall ikke kommer til å bli benyttet her. Eksempelvis har UK en momsrate på 17,5%.

 

Edit: Ved nærmere gjennomsyn tror jeg du kan slippe subqueries også

Endret 27. januar 2006 av roac

[Manfred]

Jeg har hatt gjennomsyn av denne av en meget erfaren T-SQL-programmerer som anbefalte denne løsningen. Enten må det gjøres på denne måten, eller så må jeg gjøre mange beregninger serverside etter at jeg har hentet dataene fra databasen, og dette er ennå mer ressurskrevende. Så spørsmålet er hvor man skal legge jobben. Så klart kunne jeg endret alle dataene som ligger i systemet, men hele regnskapet mitt ligger der, og jeg gidder rett og slett ikke Jeg jobber etter modellen DTSTTCPW (Do The Simplest Thing That Could Possibly Work)

 

Den designfeilen kan rettes på 2 sec, men så langt jeg vet er det en "politikk" her i landet å ikke bruke noe annet enn heltall som momsrate.

 

Den store "designfeilen" er å putte inn ink.mva i stedet for grunnlaget. Grunnen til dette igjen er at jeg utviklet dette før jeg ble mva-registrert hehe

[roac]

Jeg har hatt gjennomsyn av denne av en meget erfaren T-SQL-programmerer som anbefalte denne løsningen. Enten må det gjøres på denne måten, eller så må jeg gjøre mange beregninger serverside etter at jeg har hentet dataene fra databasen, og dette er ennå mer ressurskrevende.

5509579[/snapback]

Unnskyld meg, men jeg legger ikke mer beregninger på serveren med min løsning, akkurat de samme beregningene gjøres. Men, jeg foreslår en fjerning av cursor (som er ressurskrevende siden den bruker tempdb databasen) og fjerning av midlertidig tabell (som er ressurskrevende siden den bruker tempdb databasen). All den tid det kun er snakk om å fjerne tunge operasjoner, og ikke legge til operasjoner, så er det en ren gevinst.

 

Hvem som har anbefalt denne løsnignen for deg vet jeg ikke, men noen god løsning er det så definitivt ikke. Jeg håper du kan gi vedkommende et hint om at blant artikklene How to Perform SQL Server Row-by-Row Operations Without Cursors, SQL Server Temp Table Performance Tuning Tips og Performance Tuning SQL Server Cursors vil være nyttige for ham, og for deg

[Manfred]

Poenget er at jeg etterpå skal gjøre de grupperte uttrekkene og ha en sum... det gjør det vanskelig å gjøre det direkte. Når jeg i tillegg skal gjøre tre forskjellige ting, avhengig av parametre, er dette den greieste løsningen etter min mening.

 

Som sagt, ligger alternativet i enten å la db gjøre all denne prosesseringen, eller overlate det til web servicen... sistnevnte går ikke raskt nok.

[roac]

Poenget er at jeg etterpå skal gjøre de grupperte uttrekkene og ha en sum... det gjør det vanskelig å gjøre det direkte. Når jeg i tillegg skal gjøre tre forskjellige ting, avhengig av parametre, er dette den greieste løsningen etter min mening.

 

Som sagt, ligger alternativet i enten å la db gjøre all denne prosesseringen, eller overlate det til web servicen... sistnevnte går ikke raskt nok.

5509700[/snapback]

Hvis du mener det er et problem vil jeg snarere råde deg til å sette deg ned med en bok i Transact-SQL enn å fortsette med det prosjektet du holder på med, for det er store mangler i kunnskapen, såpass store at du ikke klarer å forstå de grove linjene i innlegget mitt engang.

 

Hadde jeg hatt koden din med litt sampledata så kunne jeg gjort jobben for deg. En time er et hav av tid for å løse problemet ditt, og få en god og effektiv løsning.

 

Dagens lille anbefaling anbefaling fra undertegnede blir å finne seg en bok om programmering av stored procedures med transact sql.

[Manfred]

Poenget mitt er at jeg har ingen behov for å gjøre den mer effektiv, da den fungerer mer enn bra nok for mitt bruk. Jeg tror ikke du leser hva jeg skriver heller

[roac]

Poenget mitt er at jeg har ingen behov for å gjøre den mer effektiv, da den fungerer mer enn bra nok for mitt bruk. Jeg tror ikke du leser hva jeg skriver heller

5510707[/snapback]

Jeg så såpass at det ikke var effektivt nok å gjøre det på webserveren, og ser at løsningen du har valgt for å løse det på databaseserveren er direkte dårlig med tanke på ytelse. Det var derfor kanskje litt naivt av meg å tro at ytelse faktisk var et problem?

 

Uansett, jeg gidder ikke dette her mer. Jeg bare håper at det ikke er noe du selger. Det kan så være at dette ikke er noen problem for deg, men dersom det går andre databaser på den samme serveren vil de påvirke hverandre, siden alle databaser bruker samme tempdb.

 

Og forresten, med tanke på å gjøre det så enkelt som mulig som du selv nevner... Enkle spørringer mot løsnigner som inkluderer både midleritidige tabeller og cursorer i tillegg til enkle spørringer? I mitt hode er i hvert fall det første alternativet det enkleste.

 

Jeg sier bare over og ut for min del, men du må gjerne ta kontakt dersom det skulle være noe, og du er interessert i konstruktive tilbakemeldinger.

[roac]

Bare en vennlgisinnet kontrollsjekk, men hvis du lager faktura i denne løsnigen din også så håper jeg at du i det minste har fått med deg at du ikke bør bruke identity til generere fakturanummer? Hvis du gjør det kan både du og evt. kunder få problemer før du/de ønsker det.

[Manfred]

Jeg lager fakturanummerne selv...

[roac]

Jeg lager fakturanummerne selv...

5512713[/snapback]

Det var godt å høre