Alternative for Loop at - Collect

[Former Member]

Hi All,

I have an internal table which consist of [27166x24(336)]Standard Table, and need to loop - collect to got the sum of the dmbtr field.

Is there any alternative to get the sum since looping took so long (and it just the test data).

I tried put sum on the query but unable due to FAE.

Have put the table query and loop below, thanks in advance !

* Query

SELECT tbs~kunnr tbs~augdt tbs~augbl tbs~gjahr tbs~belnr tbs~budat
          tbs~bldat tbs~waers tbs~xblnr tbs~blart tbs~monat tbs~shkzg
          tbs~gsber tbs~dmbtr tbs~wrbtr tbs~zfbdt tbs~zterm tbs~rebzg tbs~vbeln
          tbs~xragl tbk~bktxt tbk~stblg tbk~stjah tbk~xreversal
   APPENDING TABLE t_paym FROM bsid AS tbs
      INNER JOIN bkpf AS tbk
              ON tbk~bukrs = tbs~bukrs
             AND tbk~belnr = tbs~belnr
             AND tbk~gjahr = tbs~gjahr
     FOR ALL ENTRIES IN t_itmrv
           WHERE tbk~bukrs = p_bukrs
             AND tbs~rebzg EQ t_itmrv-belnr
             AND tbs~rebzj EQ t_itmrv-gjahr 
AND tbs~kunnr EQ t_itmrv-kunnr.

*Loop

LOOP AT t_paym.
     lt_paym-kunnr = t_paym-kunnr.
     lt_paym-vbeln = t_paym-rebzg.
     lt_paym-amt   = t_paym-dmbtr.
     COLLECT lt_paym.
ENDLOOP.

[alok_patra]

Good time for a HANA Database

[Former Member]

Hi Alok,

Care to share the difference on HANA for this case?

[ziolkowskib]

Personally I would use field symbol (LOOP ... ASSIGNING <>).

Then I would introduce temporary table in which the aggregation is stored. You read temporary table and if record is found you simply sum the fields you are interested in or otherwise you append new record.

[Former Member]

Hi Bartosz,

Thanks for your reply.

I'm sorry if i'm mistaken, but doesn't it require more looping (for the t_paym and temp table?)

Thanks.

[ziolkowskib]

What I was having in mind was to use additional tables with aggregated entries as in the following example:

DATA lt_data TYPE STANDARD TABLE OF something.

DATA lt_data_aggr LIKE lt_data[].

FIELD-SYMBOL <ls_data> LIKE LINE OF lt_data[].

FIELD-SYMBOL <ls_data_aggr> LIKE LINE OF lt_data_aggr[].

SORT lt_data[] BY key1.

LOOP AT lt_data[] ASSIGNING <ls_data>.

READ TABLE lt_data_aggr[] ASSIGNING  <ls_data_aggr> WITH KEY ke1 = <ls_data>-key1 BINARY SEARCH.

IF sy-subrc = 0.

     <ls_data_aggr>-value = <ls_data_aggr>-value + <ls_data>-value.

ELSE.

     APPEND <ls_data> TO lt_data_aggr[].

ENDIF.

ENDLOOP.

[matt]

This method will only be quicker than COLLECT if you use STANDARD tables. If you use a SORTED table, or even better, a HASHED table, COLLECT will be the optimum solution. (If you can't use aggregate SUM on the select of course!).

It really saddens me that more than fifteen years after they were introduced, there are still people who just don't use the correct table types in their programs.

[ziolkowskib]

Being perfectly honest I was not aware that hash tables and COLLECT statement are perfect match. Actually I will adjust the report I'm currently developing and will give it a try. Thanks!

[SimoneMilesi]

1. Place Kunnr and rebzg as first 2 fields
2. Sort the table by Kunnr and rebzg.
3. Use break statements (AT NEW... AT END..)
   
   

[matt]

1) Stop using tables with header lines. They're obsolete and BAD programming.

2) Rewrite your SQL so that you use a JOIN instead of FAE (if possible)

3) If 2) notpossible, define IT_PAYM as a HASHED table with appropriate unique key. HASHED tables were almost invented for use with COLLECT.

[Sandra_Rossi]

COLLECT internally creates a temporary hash table (existed before the hashed tables by the way), so I think there is a very small performance difference with using a native hashed table.

[Sandra_Rossi]

Your LOOP AT should be very quick (matter of microseconds), did you do some time measurements?

Can you share the definition of lt_paym with us?

t_paym.

[Former Member]

Hi Sandra,

Thanks for the reply,

I have change the structure to Hashed Table (as code below),

but still gets 389.155.432 MicroSec, which for processing loop of [27166x24(336)]Standard Table.

Believe its still too slow, isn't it?

Thanks in advance

* Tipe of Hashed Table / lt_paym_hash

            kunnr TYPE kunnr,
            vbeln TYPE vbeln_vf,
            amt   TYPE dmbtr,

* Tipe of t_paym

          kunnr TYPE kunnr,       
          augdt TYPE augdt,       
          augbl TYPE augbl,       
          gjahr TYPE gjahr,       
          belnr TYPE belnr_d,    
          budat TYPE budat,      
          bldat TYPE bldat,     
          waers TYPE waers,       
          xblnr TYPE xblnr,
          blart TYPE blart,
          monat TYPE monat,
          shkzg TYPE shkzg,
          gsber TYPE gsber,
          dmbtr TYPE dmbtr,
          wrbtr TYPE wrbtr,
          zfbdt TYPE dzfbdt,
          zterm TYPE dzterm,
          rebzg TYPE rebzg,
          vbeln TYPE vbeln_vf,
          xragl  TYPE bsid-xragl,
          bktxt TYPE bktxt,
          stblg  TYPE bkpf-stblg,
          stjah  TYPE bkpf-stjah,
          xreversal TYPE bkpf-xreversal,

* Loop

LOOP AT t_paym ASSIGNING <fs_paym>.
     lw_paym_hash-kunnr = <fs_paym>-kunnr.
     lw_paym_hash-vbeln = <fs_paym>-rebzg.
     lw_paym_hash-amt   = <fs_paym>-dmbtr.             
     COLLECT lw_paym_hash INTO lt_paym_hash.            
ENDLOOP.

* Trace Result (the event which consumed 88,50% is mostly consist of above loop)

[Sandra_Rossi]

389 seconds is for the whole GENERATE_BASIC_LIST subroutine. I'm curious to see what code you have in this form, I suspect there's something else than just the LOOP AT!

[Former Member]

Hi Sandra and All,

Actually yes there is one more big loop in the subroutine (code as below) which take maybe half of the total time.

Look forward to hear feedback from y'all, and many thanks!

And regarding Header Lines, since the program was created by consultant so i didn't change it until it becomes necessary

FORM generate_basic_list .

   LOOP AT t_paym ASSIGNING <fs_paym>.
     lw_paym_hash-kunnr = <fs_paym>-kunnr.
     lw_paym_hash-vbeln = <fs_paym>-rebzg.
     lw_paym_hash-amt   = <fs_paym>-dmbtr.            
     COLLECT lw_paym_hash INTO lt_paym_hash.        
   ENDLOOP.

   lt_init[] = t_itmrv[].
   DELETE lt_init WHERE blart NE 'UI'.
   LOOP AT lt_init.
     wa_vbpa-vbeln = lt_init-belnr.
     wa_vbpa-netwr = lt_init-dmbtr.
     APPEND wa_vbpa TO t_vbpa.
   ENDLOOP.

   SORT t_itmrv  BY vbeln.
   SORT t_bill   BY vbeln.
   SORT t_refso  BY vbeln posnr.
   SORT t_sls    BY pernr.
   SORT lt_blart BY blart.
   SORT t_fplt BY fpltr fplnr.

   LOOP AT t_vbpa.
     CLEAR t_list.
     READ TABLE t_itmrv WITH KEY vbeln = t_vbpa-vbeln.
     IF sy-subrc = 0.

       t_list-xragl  = t_itmrv-xragl.
       READ TABLE t_bill WITH KEY vbeln = t_itmrv-vbeln BINARY SEARCH.
       IF sy-subrc = 0.
         IF t_bill-fkart = 'ZA51'. 
           CHECK t_itmrv-budat(6) = p_keydat(6).
           t_list-flag_credmem = 'CM'.

           READ TABLE t_refso WITH KEY vbeln = t_vbpa-vgbel
                                       posnr = t_vbpa-vgpos BINARY SEARCH.

           IF sy-subrc = 0.
             t_list-crmemo  = t_itmrv-vbeln.
             t_list-crdat   = t_itmrv-budat.
             t_list-cmref   = t_refso-vgbel.
             t_list-cmval   = t_vbpa-netwr.
           ENDIF.
         ELSE.
           IF t_bill-fksto EQ 'X'.
             t_list-flag_caninv = 'CI'.  
             t_list-celnr        = t_itmrv-augbl.
             t_list-celnr_budat  = t_itmrv-augdt.
           ENDIF.
         ENDIF.  "t_bill-fkart = 'ZA51'

       ENDIF.
       t_list-kunnr    = t_bill-kunnr.
       t_list-areaid   = t_bill-vkorg.   
     ELSE.
       CLEAR t_itmrv.
       READ TABLE t_itmrv WITH KEY belnr = t_vbpa-vbeln.
       CHECK sy-subrc = 0.

       t_list-kunnr = t_itmrv-kunnr.
       t_list-xragl  = t_itmrv-xragl.
       t_list-areaid   = t_itmrv-gsber. 

       IF t_list-areaid NOT IN s_vkorg.
         CONTINUE.
       ENDIF.
     ENDIF.

     READ TABLE t_sls WITH KEY pernr = t_vbpa-pernr BINARY SEARCH.
     IF sy-subrc = 0.
       t_list-kdmc = t_sls-ename.
     ELSE.
       CLEAR t_list-kdmc.
     ENDIF.

     t_list-compid   = p_bukrs.
     t_list-vbeln    = t_itmrv-vbeln.
     t_list-belnr        = t_itmrv-belnr.
     t_list-belnr_budat  = t_itmrv-budat.

     t_list-augbl    = t_itmrv-augbl.
     t_list-augdt    = t_itmrv-augdt.

     t_list-vgbel    = t_vbpa-vgbel.    
     t_list-noinv    = t_itmrv-bktxt.   
     t_list-nosewa   = t_vbpa-aubel.
     t_list-kategori = t_vbpa-parvw.

     CALL FUNCTION 'SD_PRINT_TERMS_OF_PAYMENT'
       EXPORTING
         bldat                        = t_itmrv-zfbdt
         budat                        = t_itmrv-zfbdt
         language                     = sy-langu
         terms_of_payment             = t_itmrv-zterm
       TABLES
         top_text                     = lt_top
       EXCEPTIONS
         terms_of_payment_not_in_t052 = 1
         OTHERS                       = 2.
     IF sy-subrc <> 0.
       MESSAGE ID sy-msgid TYPE 'S' NUMBER sy-msgno
             WITH sy-msgv1 sy-msgv2 sy-msgv3 sy-msgv4.
     ENDIF.
     READ TABLE lt_top INDEX 1.
     IF sy-subrc = 0.
       t_list-tgl_inv  = lt_top-hdatum.
     ENDIF.

     CLEAR t_paym.
     READ TABLE t_paym WITH KEY rebzg = t_list-belnr." BINARY SEARCH.
     IF sy-subrc = 0.
       IF t_paym-blart = 'AB'.
         READ TABLE t_paymdz WITH KEY augbl = t_paym-belnr." BINARY SEARCH.
         IF sy-subrc = 0.
           t_list-noref     = t_paymdz-belnr.
           t_list-tgl_bayar = t_paymdz-budat.
         ENDIF.
         READ TABLE lt_blart WITH KEY blart = t_paymdz-blart BINARY SEARCH.
         t_list-ketref    = lt_blart-ltext.
       ELSE.
         t_list-tgl_bayar = t_paym-budat.
         t_list-noref     = t_paym-belnr.
         READ TABLE lt_blart WITH KEY blart = t_paym-blart BINARY SEARCH.
         t_list-ketref   = lt_blart-ltext.
       ENDIF.
       t_list-tglins   = t_itmrv-augdt.
     ENDIF.

     IF t_list-tgl_bayar(6) GT p_keydat(6).
       CONTINUE.
     ENDIF.

     t_list-niltgh   = t_vbpa-kzwi1. "t_itmrv-dmbtr.
     t_list-nilinv   = t_vbpa-netwr + t_vbpa-mwsbp. 
     t_list-pernr    = t_vbpa-pernr.

     t_list-waers    = t001-waers.

     READ TABLE t_fplt WITH KEY fpltr = t_vbpa-fpltr
                                fplnr = t_vbpa-fplnr BINARY SEARCH.
     IF sy-subrc = 0.
       t_list-prdke    = t_fplt-bezeich.
     ENDIF.

     t_list-bulan    = p_keydat+4(2).
     t_list-tahun    = p_keydat(4).
     t_list-gjahr    = t_itmrv-gjahr.
     IF t_list-xragl = 'X'.
       t_list-flag_resclr = 'RC'. 
     ENDIF.

     COLLECT t_list.
   ENDLOOP

[matt]

Like I said above. There are hashed tables and sorted tables. If you correctly define your table and use READ... TABLE KEY... you'll find things run rather more quickly.

[Sandra_Rossi]

You may add SET RUN TIME ANALYZER ON and SET RUN TIME ANALYZER OFF around your LOOP AT ... ENDLOOP, use SAT with option "particular units", to make sure that the LOOP AT is very quick, and then you'll be sure that the problem is due to the rest of code.

[Former Member]

Hi Sandra,

Thanks, sounds like a good idea.

I have tried to check with SAT on the subroutine alone, but dunno how to utilize the "SET RUN TIME ANALYZER ON".

I tried the /ron but got "No Measurement Active" notif.

Could you please help to enlight more on how to check the loop only?

Thanks

[Sandra_Rossi]

Did you read the ABAP documentation (link provided above?)

SET RUN TIME ANALYZER ON.

LOOP AT ...

  ...

ENDLOOP.

SET RUN TIME ANALYZER OFF.

In SAT, create a variant, with checkbox "particular units" checked.

Run your program from within SAT, using that variant.

When the program is over, you may see the measurements from SAT.

Don't forget to remove the SET RUN TIME ANALYZER lines.

Instead of using SET RUN TIME ANALYZER, you could of course use /RON and /ROFF but you should then do it in debug, and you must switch to the old debugger I think (with the "new" two-process debugger I think /RON would apply to the debugger session itself).

[former_member186746]

Hi,

I agree with Sandra, this should not take so long to execute.

Quick and dirty method to find the culprit is simply debugging it and pressing F5 or F6 to execute functionalities.

Also stop using header lines. If you're ever subjected to a code review you will fail.

Kind regards, Rob Dielemans

[Former Member]

Fox,

The fields (KUNNR, REBZG, DMBTR) that you use for totalling up comes only from BSID.

Considering that, can't you try running additional SELECT on BSID table only? May be the number of records would be lesser???

-Amit.

[Jelena]

+1 to all above. Get rid of BINARY SEARCH + SORT and use the correct table type. Also check what's in that FM you're calling in a loop and see if you could accomplish the same in a different way.

Not sure what's in that t_itmrv table, but it's read twice, so maybe that table should be main loop instead or you could change it's structure. But it's hard to tell from the fragment what it's all about.