Стиль написания SQL запросов

Правила раздела: faq.php?mode=okay
Модератор: Модераторы

makki M
makki M
Репутация: 199
Сообщения: 697
Зарегистрирован: 12.08.2016
С нами: 7 лет 7 месяцев
Откуда: Киев
Сайт

Сообщение #1 makki » 03.04.2019, 06:07

В новых версиях (начиная с 2.3.0) появился новый "нечеловеческий" стиль написания SQL запросов, который никак не похож на все остальные 99% запросов в движке. Более того программисту в такой каше будет тяжело разобраться. Большая просьба к разработчикам обратить внимание на это.
Привожу кусок подобного кода с файла api/FeaturesValues.php

Код: Выделить всё

 if ($count) {
            $select = $this->db->placehold("COUNT(DISTINCT `fv`.`id`) AS `count`");
            $sql_limit = "";
        } else {
            $select = $this->db->placehold("
                MAX(`fv`.`id`)            AS `id`,
                MAX(`fv`.`feature_id`)    AS `feature_id`,
                MAX(`fv`.`position`)      AS `position`,
                count(`pf`.`product_id`)  AS `count`,
                MAX(`f`.`id`)             AS `feature_id`,
                MAX(`f`.`auto_name_id`)   AS `auto_name_id`,
                MAX(`f`.`auto_value_id`)  AS `auto_value_id`,
                MAX(`f`.`url`)            AS `url`,
                MAX(`f`.`in_filter`)      AS `in_filter`,
                MAX(`f`.`yandex`)         AS `yandex`,
                MAX(`f`.`url_in_product`) AS `url_in_product`,
                MAX(`fv`.`to_index`)      AS `to_index`,
                $lang_options_sql->fields,
                $lang_features_sql->fields");
            $group_by = $this->db->placehold("GROUP BY `fv`.`id`");
            $order_by = $this->db->placehold("ORDER BY `f`.`position` ASC, `fv`.`position` ASC, `value` ASC");
        }

        $query = $this->db->placehold("SELECT
                $select
            FROM `__features_values` AS `fv`
            LEFT JOIN `__features` AS `f` ON `f`.`id`=`fv`.`feature_id`
            LEFT JOIN `__products_features_values` AS `pf` ON `pf`.`value_id`=`fv`.`id`
            $lang_options_sql->join
            $lang_features_sql->join
            $product_join
            $category_id_filter
            $variant_join
            $currency_join
            WHERE
                1
                $feature_id_filter
                $product_id_filter
                $visible_filter
                $brand_id_filter
                $features_filter
                $yandex_filter
                $other_filter
                $value_filter
                $translit_filter
                $id_filter
                $keyword_filter
                $price_filter
            $group_by
            $order_by
            $sql_limit
        ");
Cоздание и расширение функционала интернет-магазина на платформе OkayCMS 2 (с 3-й и 4-й версией не работаю)

zyxer M
zyxer M
Возраст: 32
Репутация: 77
Сообщения: 419
Зарегистрирован: 03.02.2016
С нами: 8 лет 1 месяц
Откуда: Днепр

Сообщение #2 zyxer » 03.04.2019, 11:41

А в чем "Нечеловечность"? )) Надеюсь вы хоть не за использование backticks)
Здесь разделение на

Код: Выделить всё

$select = $this->db->placehold("COUNT(DISTINCT `fv`.`id`) AS `count`");

и

Код: Выделить всё

$select = $this->db->placehold("
                MAX(`fv`.`id`)            AS `id`,
                MAX(`fv`.`feature_id`)    AS `feature_id`,
                MAX(`fv`.`position`)      AS `position`,
                count(`pf`.`product_id`)  AS `count`,
                ...

более чем очевидно, get_features_values и count_features_values объедены в один метод. Исчезает дублирование кода когда эти два метода на 90% схожи но их оба нужно поддерживать.

По поводу ф-ции MAX() вам лучше почитать об ONLY_FULL_GROUP_BY и Агрегатных ф-циях MySQL.

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

Есть здесь еще что-то "Нечеловеческое" о чем я не упомянул? ))
Всё сказанное мной, является лично моим мнением, и не является официальной позицией OkayCMS

korshunov
korshunov
Репутация: 146
Сообщения: 1854
Зарегистрирован: 03.12.2015
С нами: 8 лет 3 месяца
Skype

Сообщение #3 korshunov » 03.04.2019, 14:05

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

В запросе сходу видно два алиаса с одинаковым именем AS `feature_id`. А в реальности есть еще и третий замаскированный. Это как минимум неэкономно. И уж никак не по-человечески...

zyxer писал(а):По поводу ф-ции MAX() вам лучше почитать об ONLY_FULL_GROUP_BY и Агрегатных ф-циях MySQL.

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

Изменения-то в агрегатных ф-циях MySQL сделаны для того, чтоб код писали более правильный и логичный, а тут все сделано, чтоб разработчикам было полегче гнать морально устаревший код, а не обучаться и повышать свою квалификацию.

Все это совсем не по-человечески, халтура это откровенная...

zyxer M
zyxer M
Возраст: 32
Репутация: 77
Сообщения: 419
Зарегистрирован: 03.02.2016
С нами: 8 лет 1 месяц
Откуда: Днепр

Сообщение #4 zyxer » 04.04.2019, 07:50

Что вы имеете ввиду под фразой "корректный правильный код"? Я имею ввиду не размытую фразу которыми вы любите бросаться, а что-то конкретное.
Можно не код, но что он должен делать и как он должен работать чтобы вы могли сказать что это "корректный правильный код".

Смысл контроля - чтобы не выводить значения, которые носят случайный характер и чаще всего бессмысленны.

Это не совсем так! Смысл контроля скорее в том, чтобы явно указать MySQL какие данные вам нужно достать.
В случае если вам нужно действительно "случайные" данные (не важно какие), в 5,7 появилась ф-ция ANY_VALUE() https://dev.mysql.com/doc/refman/5.7/en/miscellan ... ctions.html#function_any-value
Но при её использовании пропадает обратная совместимость с более ранними версиями MySQL.
По этому применили ф-цию MAX() которая в нашем случае достанет нам те же данные которые нам нужны.
(кстати вы это тоже предлагали в топике viewtopic.php?f=5&t=882&p=4478#p4478)

набабахали везде MAX, чтобы ошибки явной не было
Попробуйте погонять этот запрос на разных наборах данных, и увидите что большинство из них там действительно нужны. Да, для некоторых полей они добавлены "на всякий случай".
Поясню: теоретически будет набор данных на котором и те поля нужно будет оборачивать в агрегатные ф-ции, но практически еще не встречали.

два алиаса с одинаковым именем AS `feature_id`. А в реальности есть еще и третий замаскированный
Тут полностью согласен, недоработка (точнее переработка :) ). Это нужно учесть и исправить. Но это не является чем-то сверхстрашным.
Всё сказанное мной, является лично моим мнением, и не является официальной позицией OkayCMS

korshunov
korshunov
Репутация: 146
Сообщения: 1854
Зарегистрирован: 03.12.2015
С нами: 8 лет 3 месяца
Skype

Сообщение #5 korshunov » 04.04.2019, 11:34

zyxer писал(а):Попробуйте погонять этот запрос на разных наборах данных, и увидите что большинство из них там действительно нужны. Да, для некоторых полей они добавлены "на всякий случай".

Хорошо, пробуем.
На свежеустановленной CMS открываю страницу категории Детские игрушки
http://localhost/OkayCMS232/catalog/detskie-igrushki
Запрос на этой странице имеет вид:

Код: Выделить всё

      SELECT
                MAX(`fv`.`id`)            AS `id`,
                MAX(`fv`.`feature_id`)    AS `feature_id`,
                MAX(`fv`.`position`)      AS `position`,
                count(`pf`.`product_id`)  AS `count`,
                MAX(`f`.`id`)           AS `feature_id`,
                MAX(`f`.`auto_name_id`)   AS `auto_name_id`,
                MAX(`f`.`auto_value_id`)  AS `auto_value_id`,
                MAX(`f`.`url`)            AS `url`,
                MAX(`f`.`in_filter`)      AS `in_filter`,
                MAX(`f`.`yandex`)         AS `yandex`,
                MAX(`f`.`url_in_product`) AS `url_in_product`,
                MAX(`fv`.`to_index`)      AS `to_index`,
                MAX(l.value) AS value, MAX(l.translit) AS translit, MAX(l.feature_id) AS feature_id,
                MAX(lf.name) AS name
            FROM `ok_features_values` AS `fv`
            LEFT JOIN `ok_features` AS `f` ON `f`.`id`=`fv`.`feature_id`
            LEFT JOIN `ok_products_features_values` AS `pf` ON `pf`.`value_id`=`fv`.`id`
            LEFT JOIN ok_lang_features_values l ON l.feature_value_id=fv.id AND l.lang_id = 1
            LEFT JOIN ok_lang_features lf ON lf.feature_id=f.id AND lf.lang_id = 1
            LEFT JOIN `ok_products` AS `p` ON `p`.`id`=`pf`.`product_id`
            INNER JOIN `ok_products_categories` `pc` ON `pc`.`product_id`=`pf`.`product_id` AND `pc`.`category_id` in('10','11','9')
           
            WHERE
                1
                AND `fv`.`feature_id` in('10','42','43','44','45','46','47')
                 
                AND `visible`=1
               
            GROUP BY `fv`.`id`
            ORDER BY `f`.`position` ASC, `fv`.`position` ASC, `value` ASC


1. Первое поле MAX(`fv`.`id`) AS `id` стоит под функцией MAX, которая совсем не нужна по причине имеющейся группировки как раз по этому полю. И в двух следующих полях MAX - тоже лишняя деталь.
2. Про второе поле и еще два с ним связанных (с алиасом AS `feature_id`) уже говорилось - явное безобразие (или нерациональность, или ошибка, или неграмотность - выбирайте на свой вкус).
3. Поля url_in_product - непонятно зачем и для чего тут нужно. По моим сведениям, на фронтенде оно используется в шаблоне только на странице отдельного товара. А на всех прочих страницах оно совсем ни к чему. Если я что не доглядел, ткните носом, пожалуйста...
4. Поле yandex - аналогично непонятно, зачем. я вообще не вижу не только, где его значение в результатах запроса используется не только на странице категории, но и вообще на фронтенде. Если знаете, подскажите...
5. Дальше уж анализировать не стал...

zyxer писал(а):...большинство из них там действительно нужны. Да, для некоторых полей они добавлены "на всякий случай".

Думаю, что все как раз наоборот - очень много полей просто не нужны и добавлены именно "на всякий случай".
Подозреваю, что сами Вы не особо-то пробовали "погонять этот запрос на разных наборах данных", потому как не увидеть ошибку с тройным повторный алиасом ну никак не возможно...


zyxer писал(а):Что вы имеете ввиду под фразой "корректный правильный код"? Я имею ввиду не размытую фразу которыми вы любите бросаться, а что-то конкретное.

Конкретно уже говорил: надо бы писать код под новые стандарты MySQL, а не подгонять старые методы, чтобы они работали в виде исключений. В данном примере - поголовное искусственное применение MAX в извлекаемых полях - выглядит явно нерационально. И набор полей - явно перегружен лишними значениями.

Кстати, этот запрос у меня по времени выполнения - самый затратный (съедает 25-30% от времени выполнения всех запросов на странице).


Название раздела: Предложения по улучшению OkayCMS
Правила раздела: faq.php?mode=okay

Быстрый ответ


Введите код в точности так, как вы его видите. Регистр символов не имеет значения.
Код подтверждения

   

Вернуться в «Предложения по улучшению OkayCMS»

Кто сейчас на форуме (по активности за 5 минут)

Сейчас этот раздел просматривают: 12 гостей