Я совсем дурак, или шансы еще есть?

ptr

Случилось так, что по должности я должен принимать результаты работы одного ABAP-ера. ABAP я знаю плохо, хотя уже больше 25 лет программировал на более чем двух десятках языках.
Вот смотрю я в этот код и с самого начала вижу, с моей точки зрения, самый неоптимальный способ решения проблемы. Причем программер в ответ мне говорит, что "condition which i put ... best possible way".

Задача: извлекать записи из VBAK с фильтром по Sales Organization и с датой/временем создания или модификации болшим или равным, чем в параметрах.
Есть три параметра:
l_zfdate - дата в формате YYYYMMDD
zftime - время в формате HHMMSS
vkorg - Sales Organization

Код, переданный мне:

Code:

  l_ztdate = SY-DATUM.
  l_zttime = SY-UZEIT.

* Build Where Condition for Date and Time Selection
  CLEAR l_condval.
  CONCATENATE 'erdat BETWEEN l_zfdate' 'AND l_ztdate'
                           INTO l_condval SEPARATED BY SPACE.
  IF l_zfdate = l_ztdate.
    CONCATENATE l_condval 'AND erzet BETWEEN zftime AND l_zttime'
                          INTO l_condval SEPARATED BY SPACE.
  ENDIF.

* Get Data for VBAK table for Given Date, Time and Sales Organisation
* Get Table Structure for VBAK
  PERFORM get_table_fields TABLES: vbak_nametab
                                   vbak_tab
                           USING 'VBAK'.

  REFRESH i_vbak.
  CLEAR i_vbak.
  TRY.
      SELECT *
         APPENDING CORRESPONDING FIELDS OF TABLE i_vbak
         FROM vbak
         WHERE vkorg EQ vkorg     AND
               (l_condval).

      LOOP AT i_vbak ASSIGNING <fs_vbak>.
        IF l_zfdate <> l_ztdate.
          IF <fs_vbak>-erdat EQ l_zfdate.
            CHECK <fs_vbak>-erzet BETWEEN zftime AND c_detime.
          ELSEIF <fs_vbak>-erdat EQ l_ztdate.
            CHECK <fs_vbak>-erzet BETWEEN c_dbtime AND l_zttime.
          ENDIF.
        ENDIF.
        PERFORM cont_data_values USING <fs_vbak>.
        APPEND l_line  TO vbak_tab.
        CLEAR l_line.
      ENDLOOP.

Код, который бы написал я:

Code:

  REFRESH i_vbak.
  CLEAR i_vbak.
  TRY.
      SELECT *
         APPENDING CORRESPONDING FIELDS OF TABLE i_vbak
         FROM vbak
         WHERE vkorg EQ vkorg     AND
                  erdat >= l_zfdate  AND
                  ( erdat > l_zfdate OR erzet >= zftime ) .
      LOOP AT i_vbak ASSIGNING <fs_vbak>.
        PERFORM cont_data_values USING <fs_vbak>.
        APPEND l_line  TO vbak_tab.
        CLEAR l_line.
      ENDLOOP.

Или я совсем уже на старости лет отупел и зря на программера наехал?

Zharik

Вам это так принципиально? То, что Вы делаете, называется перфекционизм. Если нет проблем с временем полнения или потребляемой, то и задумываться не надо.
Оттачивать своё мастерство можно постоянно, но, зачастую, коллеги люди плохо воспринимают, когда критикуют их код.
Зачем Вам это?

sibrin

Zharik написал(а):

Зачем Вам это?

Если у человека в должностные обязанности входит проверка чужого кода, то, наверное, это кому-то надо? Наверное, кто-то эти разработки поддерживать и развивать будет.

С текущими датой и временем сравнивать, конечно, большого смысла нет. Использование констант c_dbtime и c_detime непонятно, но вполне может быть они приводят к неправильной работе алгоритма.
Динамическое условие WHERE приводит к потере производительности (~20%): уж лучше написать два разных селекта, если этого требует оптимизация двух разных случаев.

Bully

Главное не "наехать", а "конструктивно раскритиковать".

Ибо неконстуктивная критика подобна понту.

ptr

Проблема именно со временем выполнения. Это часть скрипта для извлекания заказов в CRM каждый час из большой корпроративной базы.
И коллеги из Сент-Льюиса сильно беспокоятся о загрузке сервера.

Объяснить я пытался помягче:

I tested your changes in DEV.
All looking fine, except some performance decrease.

I looked in your code and found, that you made great work.
In case when parameter zfdate is not equal to SAP application server date, you select all records using date fiter, but not use time filter during select. Time filter will applied only during table copy loop.

My proposal is to change code from:

Code:

WHERE vkorg EQ vkorg     AND
                   erdat >= l_zfdate  AND
                   erzet >= zftime.

to:

Code:

WHERE vkorg EQ vkorg AND
                  erdat >= l_zfdate  AND
                  ( erdat > l_zfdate OR erzet >= zftime ) .

How do you think? May be it's more simple solution?

На что получил ответ, что мой код неправильный и я ничего в глобальных ERP системах не понимаю...

Вот я и сижу в сомнениях, может быть я действительно бред написал. Может, например, использование скобок в WHERE приводит к трехкратному падению производительности?

Сашок

После часа дня пятницы надо плавно готовить свой моск к выходным

Марина

Транзакция se30 - Анализ времени выполнения. Запустите свой код и код вашего программиста и сразу станет ясно кто прав кто виноват.

sibrin

ptr написал(а):

Вот я и сижу в сомнениях, может быть я действительно бред написал. Может, например, использование скобок в WHERE приводит к трехкратному падению производительности?

Да не, я специально проверял: индекс по erdat используется. Только вот проверьте (через se11), существует ли он существует в БД. У нас на всех системах в SAPе индекс определён, а в БД — нет.

Удав

При внимательном рассмотрении разработчик прав:
1.Конструкция BETWEEN обрабатывается оптимизатором ORACLE лучше, чем ">="
2.По поводу обработки "OR" то же самое - лучше его не применять по возможности.
3.Чтение текущего времени перед обработкой запроса связано с тем, чтобы зафиксировать время очередного выполнения задания, т.к. во время чтения из БД может создатся новые заказы, которых не было во время запуска выборки.

4.Динамическое WHERE в данном конкретном случае не так сильно влияет на скорость выполнения запроса (уж точно не на 20% :shock:

), как отсутствие индекса по MANDT-ERDAT-ERZET.

5.Обработка в LOOP связана с тем, что в подпрограмму передается дата и время ПРОШЛОГО запуска программы.
В случае обработки данных за предыдущие дни:
для первого дня нужно взять записи с временем от zftime до 23:59:59, для текущего дня - от начала дня до времени запуска программы.

ptr

Удав написал(а):

При внимательном рассмотрении разработчик прав:
1.Конструкция BETWEEN обрабатывается оптимизатором ORACLE лучше, чем ">="
2.По поводу обработки "OR" то же самое - лучше его не применять по возможности.
3.Чтение текущего времени перед обработкой запроса связано с тем, чтобы зафиксировать время очередного выполнения задания, т.к. во время чтения из БД может создатся новые заказы, которых не было во время запуска выборки.

4.Динамическое WHERE в данном конкретном случае не так сильно влияет на скорость выполнения запроса (уж точно не на 20% :shock:

), как отсутствие индекса по MANDT-ERDAT-ERZET.

5.Обработка в LOOP связана с тем, что в подпрограмму передается дата и время ПРОШЛОГО запуска программы.
В случае обработки данных за предыдущие дни:
для первого дня нужно взять записи с временем от zftime до 23:59:59, для текущего дня - от начала дня до времени запуска программы.

Спасибо огромное за вразумительный ответ!

1. Учту на будущее
2. Специально делал запрос с учетом этого. Чтобы сначала БД срубила по ERDAT, а потом уже фильтровала по VKORG и ERZET
3. Для данной задачи не интересно.
4. Индекс по MANDT-ERDAT есть. Я бы тут скорее об индексе по MANDT-VKORG-ERDAT подумал, так как сбытовых организаций в этой базе больше, чем заказов на продажу в одной организации за день.
5. Вот это меня и смущает. Неужели фильтрация в цикле может быть быстрее, чем фильтрация в выражении WHERE?

Желаю удачи!

Skif

ptr написал(а):

Случилось так,

1. прежде чем задумываться поглядите тексты "родныx" sap ФМ - там порой такие чудные вещи встречаются ))
2. Написание оптимальных продуктов не является признаком ума. Это устаревшее мнение.

p.s. Так что лучше свой код никому не показывайте.

sibrin

Удав написал(а):

1.Конструкция BETWEEN обрабатывается оптимизатором ORACLE лучше, чем ">="

Когда правая граница не ограничивает выборку, т.е. интервал открытый, то разницы нет. Не поленился проверить.

Удав написал(а):

2.По поводу обработки "OR" то же самое - лучше его не применять по возможности.

Это хорошо известная истина. Но одно простое сравнение в БД, конечно, дешевле, чем передача лишних данных на сервер приложений. А если ещё потом циклы крутить, то в разы медленнее получается.

Code:

data: begin of lt_mblnr occurs 0 
    , mblnr type mkpf-mblnr
    , CPUTM type mkpf-CPUTM
, end of lt_mblnr .
field-symbols: <m> like line of lt_mblnr.

select mblnr cputm
  into table lt_mblnr
  from mkpf
  where budat between '20080101' and '20080404'.

loop at lt_mblnr assigning <m>.
  if <m>-cputm <= '130000'.
     delete lt_mblnr.
  endif.
endloop.

Code:

data: lt_mblnr type table of mblnr.

select mblnr
  into table lt_mblnr
  from mkpf
  where budat >= '20080101' and 
  ( budat > '20080101' or CPUTM > '130000' ).

Первое у меня в 2 раза медленнее отработало.

Удав написал(а):

4.Динамическое WHERE в данном конкретном случае не так сильно влияет на скорость выполнения запроса (уж точно не на 20% :shock:

),

Ну, это я на примитивном селекте померял. Если сам селект выполняется долго, то разбор динамического WHERE пренебрежимо мал.

Удав написал(а):

5.Обработка в LOOP связана с тем, что в подпрограмму передается дата и время ПРОШЛОГО запуска программы.

Никто ж не мешает запомнить дату-время последнего полученного документа.

В общем, написано коряво, хотя по скорости уступать будет, скорее всего, не сильно.

Jelena

Не видя половины кода, честно говоря, трудно оценить его правильность... Кстати, помимо логической правильности, во многих компаниях существуют стандарты, касающиеся readability и прочая. Например, динамический WHERE тут неоправдан IMHO и одновременно ухудшает читаемость программы и повышает риск exception. Наверное следует прежде всего руководствиваться имеющимися Development Standards. Тогда и обид не будет.

ptr написал(а):

Задача: извлекать записи из VBAK с фильтром по Sales Organization и с датой/временем создания или модификации болшим или равным, чем в параметрах.
Есть три параметра:
l_zfdate - дата в формате YYYYMMDD
zftime - время в формате HHMMSS
vkorg - Sales Organization

Задача, кстати, не совсем понятна. Вам надо найти записи, которые были созданы в день >= l_zfdate и во время >= zftime, или нужно найти записи, которые были созданы позже момента времени, состоящего из совокупности и l_zfdate zftime?

Например, l_zfdate = 20040401 (1 апреля), zftime = 180000 (6 вечера). Если запись создана 2-го апреля, но, скажем, в 10 утра, то она вам нужна или нет?

У нас на таких задачах народ часто накалывается, пытаясь включить время сразу в выборку.

trop

динамика в меру - добро, просто нужно отладить пару раз.
в примере пара (дата, время) обозначают Момент (маркер на шкале t),
время в Моменте всегда привязано к указанной дате и просто
уточняет определение Момента в ее пределах,
поэтому из выборки б/д его нужно выкинуть при неодинаковой дате

вот так, похоже

Code:

between: a   (1)
         b

and: >=: a   (1+1)
     <=: b

Monarch

Jelena написала:

Задача, кстати, не совсем понятна. Вам надо найти записи, которые были созданы в день >= l_zfdate и во время >= zftime, или нужно найти записи, которые были созданы позже момента времени, состоящего из совокупности и l_zfdate zftime?

Да, это надо прояснить. Если вариант 2 (вполне стандартная задача),
то я бы, наверное, сделал 2 SELECT'a:

Code:

SELECT ... WHERE erdat = l_zfdate and erzet >= zftime

+

Code:

SELECT ... WHERE erdat > l_zfdate

+ ессно, избавиться от APPENDING CORRESPONDING FIELDS OF TABLE ...

Правила форума

Я совсем дурак, или шансы еще есть?

Кто сейчас на конференции