03-13-2009 3:36 AM
Hi all,
I have developed this coding for a domestic material wise sales report. Please comment on this coding. will it be OK performance wise? its taking more time to run in production.
****************Fetching Data from Database Tables**********************
select * into corresponding fields of table it_vbrk
from vbrk
where vbeln in s_vbeln and
vbtyp = 'M' and
vtweg = 'TR' and
fkdat in s_fkdat.
* Fetching Customer group name details
select * into corresponding fields of table it_t151t
from t151t
where spras = 'EN'.
* Fetching price list description
select * into corresponding fields of table it_t189t
from t189t
where spras = 'EN'.
if not it_vbrk[] is initial.
* Fetching Material no, sub-family, tint and coating codes
select * into corresponding fields of table it_vbrp
from vbrp
for all entries in it_vbrk
where vbeln = it_vbrk-vbeln and
subfam in s_subfam and
codcol in s_codcol and
codcoat in s_coat and
epaiscom in s_thick.
* Fetching sold-to-party & ship-to-party info
select * into corresponding fields of table it_vbpa
from vbpa
for all entries in it_vbrk
where vbeln = it_vbrk-vbeln and
parvw in ('AG', 'WE').
endif.
it_temp_vbrp[] = it_vbrp[].
sort it_temp_vbrp by matnr.
delete adjacent duplicates from it_temp_vbrp comparing matnr.
if not it_temp_vbrp[] is initial.
* Fetching Material Description
select * into corresponding fields of table it_makt
from makt
for all entries in it_temp_vbrp
where matnr = it_temp_vbrp-matnr and
spras = 'EN'.
endif.
if not it_vbrp[] is initial.
* Fetching BED, AED, CESS, SHCESS, CST, VAT
select * into corresponding fields of table it_zinvitem
from zinvitem
for all entries in it_vbrp
where invoice = it_vbrp-vbeln and
line = it_vbrp-posnr.
* Fetching sales order document no
select * into corresponding fields of table it_vbfa
from vbfa
for all entries in it_vbrp
where vbeln = it_vbrp-vbeln and
posnn = it_vbrp-posnr and
vbtyp_n = 'M' and
vbtyp_v = 'C'.
* Fetching invoice reversal or cancellation details
select * into corresponding fields of table it_vbfa1
from vbfa
for all entries in it_vbrp
where vbelv = it_vbrp-vbeln and
vbtyp_n in ('N', 'O', 'H', 'T').
endif.
if not it_vbfa[] is initial.
* Fetching project ref, transaction no, version no data
select * into corresponding fields of table it_zcoih
from zcoih
for all entries in it_vbfa
where vbeln = it_vbfa-vbelv and
litem = it_vbfa-posnv.
endif.
if not it_zcoih[] is initial.
* Fetching Approved price
select * into corresponding fields of table it_zcoi
from zcoi
for all entries in it_zcoih
where ext_file_no = it_zcoih-ext_file_no and
vrsctsap = it_zcoih-vrsctsap and
porid = it_zcoih-porid and
posnr = it_zcoih-posnr.
* Fetching Approved price if not maintained in ZCOI table
select * into corresponding fields of table it_zcohr
from zcohr
for all entries in it_zcoih
where ext_file_no = it_zcoih-ext_file_no and
porvr = it_zcoih-vrsctsap and
porid = it_zcoih-porid and
posnr = it_zcoih-posnr.
endif.
if not it_vbpa[] is initial.
* Fetching sold-to-party & ship-to-party names
select * into corresponding fields of table it_kna1
from kna1
for all entries in it_vbpa
where kunnr = it_vbpa-kunnr.
endif.
if not it_kna1[] is initial.
* Fetching Ship-to-party destination
select * into corresponding fields of table it_knvv
from knvv
for all entries in it_kna1
where kunnr = it_kna1-kunnr.
endif.
************Populating values into final output internal table*******
loop at it_vbrp.
it_final-matnr = it_vbrp-matnr.
read table it_makt with key matnr = it_final-matnr.
if sy-subrc = 0.
it_final-maktx = it_makt-maktx.
endif.
shift it_final-matnr left deleting leading '0'.
it_final-netvalue = it_vbrp-netwr.
it_final-mwsbp = it_vbrp-mwsbp.
it_final-subfam = it_vbrp-subfam.
it_final-codcol = it_vbrp-codcol.
it_final-codcoat = it_vbrp-codcoat.
it_final-epaiscom = it_vbrp-epaiscom.
read table it_vbrk with key vbeln = it_vbrp-vbeln.
if sy-subrc = 0.
it_final-vbeln = it_vbrk-vbeln.
it_final-fkdat = it_vbrk-fkdat.
it_final-fkart = it_vbrk-fkart.
it_final-kdgrp = it_vbrk-kdgrp.
read table it_t151t with key kdgrp = it_vbrk-kdgrp.
if sy-subrc = 0.
it_final-cgname = it_t151t-ktext.
endif.
read table it_t189t with key pltyp = it_vbrk-pltyp.
if sy-subrc = 0.
it_final-pldesc = it_t189t-ptext.
endif.
it_final-pltyp = it_vbrk-pltyp.
endif.
read table it_vbpa with key vbeln = it_vbrp-vbeln parvw = 'AG'.
if sy-subrc = 0.
it_final-kunnr1 = it_vbpa-kunnr.
read table it_kna1 with key kunnr = it_vbpa-kunnr.
if sy-subrc = 0.
it_final-name1 = it_kna1-name1.
endif.
endif.
read table it_vbpa with key vbeln = it_vbrp-vbeln parvw = 'WE'.
if sy-subrc = 0.
it_final-kunnr2 = it_vbpa-kunnr.
read table it_kna1 with key kunnr = it_vbpa-kunnr.
if sy-subrc = 0.
it_final-name2 = it_kna1-name1.
endif.
read table it_knvv with key kunnr = it_vbpa-kunnr.
if sy-subrc = 0.
it_final-inco2 = it_knvv-inco2.
endif.
endif.
read table it_vbfa with key vbeln = it_vbrp-vbeln
posnn = it_vbrp-posnr.
if sy-subrc = 0.
it_final-vbelv = it_vbfa-vbelv.
select single * from vbak where vbeln = it_vbfa-vbelv.
if sy-subrc = 0.
it_final-auart = vbak-auart.
endif.
if vbak-auart = 'ZNET'. " Coater net order
* Fetching List price when coater net order
perform a625.
elseif vbak-auart = 'ZOR1' or vbak-auart ='ZTRD'. " Standard order
* Fetching List price when standard order
perform a904_905.
endif.
read table it_zcoih with key vbeln = it_vbfa-vbelv
litem = it_vbfa-posnv.
if sy-subrc = 0.
it_final-porid = it_zcoih-porid.
it_final-ext_file_no = it_zcoih-ext_file_no.
it_final-vrsctsap = it_zcoih-vrsctsap.
read table it_zcoi with key ext_file_no = it_zcoih-ext_file_no
vrsctsap = it_zcoih-vrsctsap
porid = it_zcoih-porid
posnr = it_zcoih-posnr.
if sy-subrc = 0.
it_final-netwr = it_zcoi-netwr.
else.
read table it_zcohr with key ext_file_no = it_zcoih-ext_file_no
porvr = it_zcoih-vrsctsap
porid = it_zcoih-porid
posnr = it_zcoih-posnr.
if sy-subrc = 0.
it_final-netwr = it_zcohr-netwr.
endif.
endif.
endif.
endif.
clear it_final-category.
read table it_vbfa1 with key vbelv = it_vbrp-vbeln
vbtyp_n = 'N'.
if sy-subrc = 0.
it_final-category = 'Cancelled'.
else.
clear it_final-category.
read table it_vbfa1 with key vbelv = it_vbrp-vbeln
vbtyp_n = 'O'.
if sy-subrc = 0.
it_final-category = 'Reversed'.
else.
clear it_final-category.
read table it_vbfa1 with key vbelv = it_vbrp-vbeln
vbtyp_n = 'H'.
if sy-subrc = 0.
it_final-category = 'Reversed'.
else.
clear it_final-category.
read table it_vbfa1 with key vbelv = it_vbrp-vbeln
vbtyp_n = 'T'.
if sy-subrc = 0.
it_final-category = 'Reversed'.
endif.
endif.
endif.
endif.
read table it_zinvitem with key invoice = it_vbrp-vbeln
line = it_vbrp-posnr.
if sy-subrc = 0.
it_final-bprice = it_zinvitem-basic.
it_final-bed = it_zinvitem-bed.
it_final-aed = it_zinvitem-aed.
it_final-cess = it_zinvitem-cess.
it_final-shcess = it_zinvitem-shcess.
it_final-cst = it_zinvitem-cst.
it_final-vat = it_zinvitem-vat.
it_final-freight = it_zinvitem-freight.
it_final-insr = it_zinvitem-insr.
* it_final-netvalue = ( it_final-bprice + it_zinvitem-bed +
* it_zinvitem-aed + it_zinvitem-cst +
* it_zinvitem-vat + it_zinvitem-freight ).
it_final-total = ( it_final-bprice + it_zinvitem-bed +
it_zinvitem-aed + it_zinvitem-cess +
it_zinvitem-shcess +
it_zinvitem-cst + it_zinvitem-vat +
it_zinvitem-freight + it_zinvitem-insr ).
else.
perform fetch_zinvitem.
if sy-subrc eq 0.
it_final-bprice = wa_zinvitem-basic.
it_final-bed = wa_zinvitem-bed.
it_final-aed = wa_zinvitem-aed.
it_final-cess = wa_zinvitem-cess.
it_final-shcess = wa_zinvitem-shcess.
it_final-cst = wa_zinvitem-cst.
it_final-vat = wa_zinvitem-vat.
it_final-freight = wa_zinvitem-freight.
it_final-insr = wa_zinvitem-insr.
* it_final-netvalue = ( it_final-bprice + wa_zinvitem-bed +
* wa_zinvitem-aed + wa_zinvitem-cst +
* wa_zinvitem-vat + wa_zinvitem-freight ).
it_final-total = ( it_final-bprice + wa_zinvitem-bed +
wa_zinvitem-aed + wa_zinvitem-cess +
wa_zinvitem-shcess +
wa_zinvitem-cst + wa_zinvitem-vat +
wa_zinvitem-freight + wa_zinvitem-insr ).
endif.
endif.
it_final-cst_percent = ( it_final-cst / ( it_final-bprice + it_final-bed
+ it_final-cess + it_final-shcess ) ) * 100.
* IF it_final-porid IS INITIAL.
*
* it_final-deviation = it_final-kbetr - it_final-netvalue.
*
* ELSE.
*
* it_final-deviation = it_final-netwr - it_final-netvalue.
*
* ENDIF.
if it_vbrp-vrkme = 'M2'.
it_final-totqty_sqm = it_vbrp-fkimg.
elseif it_vbrp-vrkme = 'VOL'.
it_final-totqty_sqm = ( it_vbrp-length * it_vbrp-width * it_vbrp-fkimg )
/ 1000000.
elseif it_vbrp-vrkme = 'PCK'.
it_final-totqty_sqm = ( it_vbrp-length * it_vbrp-width * it_vbrp-fkimg *
it_vbrp-codtyp+1(3) ) / 1000000.
endif.
* IF NOT it_final-totqty_sqm IS INITIAL.
* it_final-netval_totqty = it_final-netvalue / it_final-totqty_sqm.
* it_final-tot_sqm = it_final-total / it_final-totqty_sqm.
* ENDIF.
append it_final.
clear it_final.
endloop.
* Deleting duplicates in final output internal table
sort it_final by vbeln.
delete adjacent duplicates from it_final comparing all fields.
* Displaying the report output as ALV list display
end-of-selection.
if not it_final[] is initial.
perform alt_line_color. " Creating colors for alternative ALV rows
perform report_output.
* Deleting entries of all internal tables
refresh: it_vbrk, it_vbrp, it_kna1, it_zinvitem, it_vbpa, it_final,
it_t151t, it_t189t, it_vbrk1, it_temp_vbrp, it_makt,
it_knvv, it_vbfa, it_vbfa1, it_zcoih, it_zcoi, it_zcohr.
* freeing up the memories allocated to the internal tables
free: it_vbrk, it_vbrp, it_kna1, it_zinvitem, it_vbpa, it_final,
it_t151t, it_t189t, it_vbrk1, it_temp_vbrp, it_makt,
it_knvv, it_vbfa, it_vbfa1, it_zcoih, it_zcoi, it_zcohr.
else.
message 'No Records available for the selection' type 'I'.
endif.
Thanks,
Rajan
Edited by: Matt on Mar 13, 2009 8:33 AM - added tags
03-13-2009 5:27 AM
Hi:
I would advices 2 things
1. Please avoid corresponding fields , use the field list only as far as possible.
2. Sort the table before read and use Binary Search
Regards
Shashi
03-13-2009 4:59 AM
Hi Rajan,
In your program you have to tune the program so that it takes litlle bit time.
In you all quaeries you used select *, if you requied all fileds of table , then it is OK.
otherwise create structure for each table whcih contanining the fields that u want in your quarries.
For example,
Types:begin of ty_makt,
spras type makt-spras,
matnr type makt-matnr,
maktx type makt-maktx,
end of ty_makt.
data:it_makt type ty_makt,
wa_matk type ty_makt.
Like this u create internal table & work areas.
So this will reduce datbase fetching.
Also whereever requied use range
For exapmle:instead of parvw in ('AG', 'WE') , use parvw in rg_parvw like this.
use sorted table with indexing for loop, It takes little time to run loop.
for example.
it_vbrp type sorted table of ty_vbrp with non-unique key vbeln posnr matnr,
CLEAR w_tabix.
READ TABLE it_vbrp INTO wa_vbrp INDEX w_index.
IF sy-subrc EQ 0.
w_tabix = sy-tabix + w_tabix.
Vbrp loop start *******************
LOOP AT it_vbrp INTO wa_vbrp FROM w_tabix.
ur coding
endloop.
endif.
Or u can used hased tabel also.
Check ur program with se30 tcode, check whether coding takes time or datbase fetching.
Then do tune the program accordingly.
Thanks & Regards,
Anagha Deshmukh
03-13-2009 5:27 AM
Hi:
I would advices 2 things
1. Please avoid corresponding fields , use the field list only as far as possible.
2. Sort the table before read and use Binary Search
Regards
Shashi
03-13-2009 6:46 AM
I am giving your general guidelines which are followed for any program for better performance.
1. Use minimum reuqired fields in SELECT statement
2. Do not use INTO CORRESPONDING.
3. Select minimum required data by restricting the WHERE clause.
4. Try to use max. possible fields in WHERE clause which are part of primary or secondary indexes
5. Avoid nested LOOP (loopu2026 loop at where u2026 endloop u2026 endloop)
6. EQ has been specified in WHERE clause whenever possible to aid index selection.
7. Positive WHERE clauses have been specified whenever possible to aid index selection.
8. Avoid nested selects and loops through the use of dictionary VIEWs, ABAP joins or by using the FOR ALL ENTRIES addition in the SELECT statement.
9. ORDER BY is only used if the order of the fields is the same as the order of the fields in the Index used for the Table Select. Otherwise use SORT ITAB BY on the returned data.
10. Use FREE command to release the memory allocated to internal tables when the program is finished processing the data in the table.
These will be already available on forum and many more.
Regards,
Sunil
PS: Please close the thread once you are happy with all replies.
03-13-2009 7:01 AM
Hi Rajan,
As also said by others avoid using 'SELECT *' and 'INTO CORRESPONDING'.
Declare you internal table only with those fields that is required for you so that you don't have to use the select * and corresponding, like:
Data:
Begin of fs_itab,
field1 type table-field1,
field2 type table-field2,
field3 type table-field3,
field4 type table-field4,
End of fs_itab.
Data:
itab like standard table of fs_itab.
Select field1
field2
field3
field4
from table
into table itab
where ....... " give the conditions.
With luck,
Pritam.
03-13-2009 7:26 AM
Avoid using 'into corresponding fields of table' in select query.
Use into table.
This tunes the code.
Reg,
Sachin
03-13-2009 7:38 AM
In addition to the suggestions to specify the fields you need, and to use sorted tables:
1. Consider using HASHED table where you are using the internal table for look ups, especially if the key is unique, and you don't need indexed access. Otherwise use SORTED tables.
2. Use INNER JOIN rather than FOR ALL ENTRIES.
matt
03-13-2009 8:13 AM
Opposite to all others, leave the 'INTO CORRESPONDING' where it is, in 10 years performance optimization I have never seen a problem with that.
If there is already running code, then you should not check coding - code inspector can be used of course. The code inspector has all useful checks on coding, there is not check on into corresponding, the is no simple check on nested loop and nested selects, because they all are useful in some circumstances. Nested Loops with sorted or hashed tables are no problems, Nested selects with buffered tables are o.k. and so
Performance is
+ index usage
+ buffer usage
+ internal table usage
That is checked on coding, see
/people/randolf.eilenberger/blog/2009/01/21/code-inspector146s-performance-checks-iii
and two other blogs referred inside.
Even better than checing coding, is checking execution, use the usual trace SQL Trace and SE30,
as described here or somewhere else.
SQL trace:
/people/siegfried.boes/blog/2007/09/05/the-sql-trace-st05-150-quick-and-easy
SE30
/people/siegfried.boes/blog/2007/11/13/the-abap-runtime-trace-se30--quick-and-easy
You should optimize the effects with more than 5% impact and not the 0.001% effects,
of course they also improve performance, but nobody will notice it.
Siegfried
03-13-2009 8:30 AM
Hi,
Apart from above all suggestions, if possible avoid calling performs to fetch data inside Loop.......Endloop.
Fetching data inside loop...........end loop leads to performance issue as the number of database hits are high.
So try to fetch all the data outside loop..endloop and use READ TABLE with BINARY SEARCH to read the corresponding data inside the loop.
Thanks
Sankar
03-13-2009 9:36 AM
Hi Rajan,
select single * from vbak where vbeln = it_vbfa-vbelv.
if sy-subrc = 0.
it_final-auart = vbak-auart.
endif.
in this line u it looks like u need only auart from vbak table.
if so define a work area for auart and select only that field.
select single auart from vbak into wa_auart where vbeln = it_vbfa-vbelv.
if sy-subrc = 0.
it_final-auart = vbak-auart.
endif.
and also avoid into corresponding if not needed.
03-13-2009 11:58 AM
> 8. Avoid nested selects and loops through the use of dictionary VIEWs, ABAP joins or by using the
> FOR ALL ENTRIES addition in the SELECT statement.
this is only partly true, for buffered tables the SELECT inside the loop is o.k. !
* Fetching Customer group name details
select * into corresponding fields of table it_t151t
from t151t
where spras = 'EN'.
* Fetching price list description
select * into corresponding fields of table it_t189t
from t189t
where spras = 'EN'.
should not be preselected but read from the buffer! inside the loop.
Your usage of the INTO CORRESPONDING is incorrect, it is not necessary with SELECT * and
result structure which is equal to the starting structure.
Use SORTED or hashed tables.
Then you are fine,, besides a check of the SQL Trace, I can not see whether your SELECTs work fine.
Check the SQL Trace, summary, if no statement has a very large duration, then you are fine.
Siegfried
03-13-2009 12:46 PM
1.
select * into corresponding fields of table it_vbrk
from vbrk
where vbeln in s_vbeln and
vbtyp = 'M' and
vtweg = 'TR' and
fkdat in s_fkdat.
I guess this query might take more time.
Is s_vbeln always filled, IF No -> Is Secondary index for vbrk-FKDAT available?.
If No -> a.INNER join With VBRP and add vbrp-erdat IN s_fkdat in WHERE condition
(assumed that no back dated billing is done ) OR
b. Creating index for the field fkdat OR
c.Running in background
(In this case option A. might be useful,even incase of back dated billing making use of alternate logic
like vbrk-fkdat in s_fkdat and vbrp-erdat IN s_newdat which can be say s_fkdat-low - 45 or 60 )
2.
* Fetching invoice reversal or cancellation details
select * into corresponding fields of table it_vbfa1
from vbfa
for all entries in it_vbrp
where vbelv = it_vbrp-vbeln and
vbtyp_n in ('N', 'O', 'H', 'T').
vbtyp is already available in VBRK..This selection from VBFA can be avoided
Cheers
03-13-2009 12:55 PM
Hi ,
Please avoid using VBFA without key fields. I dont know why you are using the below code. Sales order numbers are already available from the first select statment it self.
Fetching sales order document no
select * into corresponding fields of table it_vbfa
from vbfa
for all entries in it_vbrp
where vbeln = it_vbrp-vbeln and
posnn = it_vbrp-posnr and
vbtyp_n = 'M' and
vbtyp_v = 'C'.
Regards,
Senthilvel P.
03-14-2009 9:52 AM
Hi Rajan,
Along with the various changes as suggested by the above replies
Make sure your read statements inside the final loop will be always added with binary search wherever possible
read table it_vbrk with key vbeln = it_vbrp-vbeln
read table it_vbrk with key vbeln = it_vbrp-vbeln BINARY SEARCH.
This improves the performance significantly.
GS
03-16-2009 6:55 AM
Hi all,
Thanks for all the suggestions!
I have awarded points for all helpful replies.
Rajan