Как правильно задать запрос UPDATE где название столбца переменная?

Question

[Михаил Смирнов]

Есть таблица со столбцами id, count1, count2,count3
Получаю массив вида {[21-1]=>{[count]=>3},[2-2]=>{[count]=>20},[5-3]=>{[count}=>15}.....}
где 21-1 это id и 1 это номер столбца для изменения значения
Написал код для внесения изменений в таблицу
Всё работает но выглядит как то топорно. как можно улучшить код и как правильно задать запрос если в качестве названия столбца использовать переменную?

$sql = "SELECT `count1`, `count2`, `count3` 
        FROM `list` 
        WHERE `id` = ?";
    foreach($data as $id =>$value ) {
        $count=$value['count'];
        $id=explode("-", $id);
        $stmt = $conn -> prepare ($sql);
        $stmt -> execute([$id[0]]);
        $result = $stmt -> fetch(PDO::FETCH_ASSOC);
        if ($id[1]== '1') {$result=(int)$result['count1']-$count; $column = 'count1';};
        if ($id[1]== '2') {$result=(int)$result['count2']-$count; $column = 'count2';};
        if ($id[1]== '3') {$result=(int)$result['count3']-$count; $column = 'count3';};
        $sql_upd = "UPDATE `list` 
            SET $column = ?
            WHERE `id` = ?";
        $stmt = $conn -> prepare ($sql_upd);
        $stmt -> execute ([$result,$id[0]]);
     }

Comments

[Виктор Кожухарь]

Я считаю, что если всё работает и хорошо читается, то не надо усложнять код.
Ну, можно, например, эти 3 if заменить одним, обработать ситуацию, когда там могут быть не только эти три цифры 1,2,3.
Можно параметры в запросе сделать именованными, а не просто в виде вопросительных знаков. Вынести объявление переменной $sql_upd наверх, рядом с $sql.

[Михаил Смирнов]

Виктор Кожухарь, Ну, можно, например, эти 3 if заменить одним, обработать ситуацию, когда там могут быть не только эти три цифры 1,2,3 - как бы вы реализовали?

[Виктор Кожухарь]

Михаил Смирнов,

<?php
// Т.к. мы подставляем переменную $column прямо в SQL код без обработки,
// то мы обязаны позаботиться о том, чтобы там не было SQL-иньекций
// Поэтому, мы должны крайне тщательно проверить, соответствует ли наше значение допутимым вариантам
// Проверяем, что там только 1, 2 или 3, а также проверяем, чтобы там не было вредоносного "продолжения"
$columnNumber = (int) $id[1];
if ($columnNumber < 1 || $columnNumber > 3 || strlen($id[1]) > 1) {
    throw new \InvalidArgumentException("Неверный номер поля");
}

$column = 'count' . $columnNumber;

[Vitsliputsli]

Для защиты от SQL-инъекций достаточно:
$column = 'count' . (int)$columnNumber;
Хотя для такой подстановки, если столбцов вменяемое кол-во, я бы лучше сделал так:

const COLUMNS = [
    1 => 'column1',
    2 => 'column2',
    3 => 'column3',
];
if (!isset(COLUMNS[$id])) {
    throw new \InvalidColumnException();
}
$column = COLUMNS[$id];

Все таки это сопоставление, и вычисление может быть не лучший вариант, а динамически составляемое имя столбца очень портит читаемость.

Но, если делать рефакторинг, то я бы в первую очередь обратил бы внимание, что за странные id с тире внутри и вообще зачем динамические столбцы.

[Михаил Смирнов]

Vitsliputsli, Спасибо.

[roswell]

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

[Михаил Смирнов]

roswell, Структура отвечает поставленной задаче. Есть объект который может быть трех видов поэтому если каждому объекту присвоить три разных id будет тоже самое но не в ширину, а в высоту и на мой взгляд менее читабельно, а так одно id одно название и три разных варианта объекта. Название столбцов взяты для примера название можно изменить на tipe1, tipe2, tipe3. Если касаемо id с тире то это только входные данные в массиве.
Хотелось бы выслушать предложения для улучшения вида таблицы с учётом изложенного выше.

[roswell]

Михаил Смирнов, объект может принимать все три вида одновременно или только один?

[Михаил Смирнов]

roswell, все три вида

[Михаил Смирнов]

Объект может быть и оного и двух и трёх видов одновременно. Для примера (упрощённо), ручка - синяя, красная, черная.

[roswell]

Михаил Смирнов, ну тогда бы я посоветовал избавиться от всех трёх столбцов count1, count2, count3, а вместо них обойтись одним типа SET (если у нас речь идёт о MySQL).

[Ипатьев]

roswell, это же счетчики, а в SET константы. Тут, как вы и говорили выше, надо нормальную связанную таблицу, и эта циферка станет обычным условием в where

[Vitsliputsli]

Михаил Смирнов, текущая схема вполне себе неплохая, но с точки зрения реляционной модели нужно сделать составной ключ, т.е. таблица будет выглядеть примерно так:
id_part1 id_part2 count
21 1 3
21 2 4
здесь первичный ключ составной id_part1 + id_part2.
В принципе, не так принципиально какой вариант выбрать, если только у вас нет повышенных требований к производительности. А если есть, то нужно исходить из запросов которые вы будете делать, а не только от хранимых данных.

[Ипатьев]

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

[Vitsliputsli]

Ипатьев, если плодить то будет печально, действительно не указал на это. Но, насколько я понял, они у автора не плодятся, а есть объект с 3 свойствами. Если есть подозрение, что будут плодиться, то очевидно, что существующий вариант не лучший.
В любом случае, выбрать наилучший вариант невозможно, не зная все о данных и вариантов обращений к ним.

[Ипатьев]

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

[roswell]

Ипатьев, то, что столбцы называются countX, необязательно что в них должны содержаться именно счётчики. Я уточнил у автора вопроса -- там, на самом деле, содержатся некоторые признаки, которые могут сочетаться.

[Ипатьев]

roswell, та ради бога, пусть сочетаются.
Теги под блог постами тоже сочетаются. Но их никто не пишет в колоночки. Для сочетания и существует связь один ко многим.

[Vitsliputsli]

Ипатьев, ага, проглядел этот момент. Очевидно, что в случае неопределенного кол-ва параметров не получится создать неопределенное кол-во столбцов. Согласен, надо было на это обратить внимание.
В общем случае, нормализация не панацея и не серебрянная пуля, а денормализация наш повседневный инструмент. Специализация может оказаться более важной, чем универсализация. Да и с данными там непонятно, там вполне может оказаться, что это все же один и тот же объект с вполне определенными свойствами, т.к. автор не сознается об истинных данных, и явно заменяет их примерами из головы, что при этом потерялось неизвестно.
И, повторюсь, проектировать бд нужно исходя из того, как ей будут пользоваться, а не только исходя из данных.
И, кстати, чем схема то плохая? Если параметров 3.

[Михаил Смирнов]

Vitsliputsli, Так я написал об истинных данных один объект, может быть представлен от одного до трех типов (вариантов) в каждом столбце информация о количестве данного объекта в данного типа (варианта). И при увеличении свойств объекта согласен с Ипатьев и его советом - Здесь нужна связанная таблица, один ко многим, и она сразу снимет все проблемы

[Ипатьев]

Vitsliputsli, нормализация - это серебряная пуля и панацея. Один из ключевых принципов архитектуры.
И не надо ставить денормализацию на одну доску с ней. Это не принцип проектирования баз данных, а компромисс, который никогда не закладывается исходно, а добавляется сильно позже, когда (если) возникают проблемы с производительностью.

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

[Vitsliputsli]

Ипатьев,

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

"серебряной пули" не существует, в этом и смысл выражения. Согласившись, что денормализация необходима, вы это подтвердили. И, напрасно думаете, что это "никогда не закладывается исходно, а добавляется сильно позже". Кладут болт на производительность только для слабонагруженных системе, при высокой нагрузке уже на старте не взлетит. Решение любой архитектурной задачи - это компромис, ваши компромисы видно связаны с жертвой производительностью, но это далеко не всегда допустимо.

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

А где говнокод? Динамический SQL? Да он присутствует практически в любом проекте, и то, что у автора он собирается не так элегантно как в популярном фреймворке, ни разу не говнокод. Как сделать боле читабельным код на php и убрать гонку я показал, но схема БД здесь не при чем. Да и называть любой не свой код говнокодом, это так себе...
И вы ничего не показали, ваш update ничем не лучше update с динамическим SQL. Пока мы больше не узнаем о том, как еще будут работать с этими данными (или как они соотносятся) бессмысленно говорить о "плохих" схемах. Про производительность и место для хранения вообще молчу, я так понял на это забиваем, но все же какую нормальную форму нарушает текущая схема?

[Ипатьев]

Vitsliputsli, так уж совсем загоняться не стоит. Не делайте вид, что не поняли, чем простой апдейт лучше чем пол-экрана пхп кода :)

[Vitsliputsli]

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

Answer 1

[Ипатьев]

Этот вопрос - прекрасная иллюстрация того факта, что нормализация базы данных - это не блажь оторванных от жизни теоретиков, а насущная необходимость. И её отсутствие приводит к проблемам на ровном месте.

Уже по наличию нумерованных столбцов сразу видно, что структура БД кривая. А текущая проблема делает это еще более наглядным: собственно, сама постановка вопроса, "как задать имя столбца через переменную", говорит о том, что имя колонки используется в условии. То есть оно должно быть значением в строке.

Здесь нужна связанная таблица, один ко многим, и она сразу снимет все проблемы, а запросы станут мягкими и шелковистыми:

UPDATE link_count SET count=count+? WHERE link_id=? and number=?

Comments

[Vitsliputsli]

Какую нормальную форму нарушает текущая реализация?
То, что вы предлагаете не нормализация, а переход к EAV, которая применяется при большой вариативности атрибутов, но небольшом их кол-ве для каждой сущности.
Когда необходим поиск по разным столбцам таблицы берут и создают запросы динамически с помощью какого-либо билдера, т.е. "задают имя столбца через переменную". Никто ради этого не переводит таблицу в формат EAV или похожий.
Пока неизвестны все варианты работы с данными однозначно зявить что нужна только какая-то конкретная схема нельзя. Так для EAV адресация будет работать медленней из-за составного ключа, объем хранимых данных также может вырасти, а не уменьшиться, а если окажется, что автору нужно выводить аттрибуты как таблицу, то привет pivot (т.е. столько join, сколько атрибутов будем выводить).
Т.е. если и нужна EAV, то точно не по указанным причинам.

[Ипатьев]

Vitsliputsli, вы бредите. Не нужно называть словом EAV любое отношение один ко многим. Теги, к которым относится этот вопрос - это, по-вашему, тоже EAV? :)
Судя по всему, вас что-то очень задело, и вы продолжаете спорить просто ради спора.

[Vitsliputsli]

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

Answer 2

[Анастасия Foxman]

Как насчет оператора CASE?

$sql = "UPDATE `list` 
        SET count1 = CASE WHEN id = ? AND ? = 1 THEN count1 - ? ELSE count1 END,
            count2 = CASE WHEN id = ? AND ? = 2 THEN count2 - ? ELSE count2 END,
            count3 = CASE WHEN id = ? AND ? = 3 THEN count3 - ? ELSE count3 END
        WHERE id = ?";

$stmt = $conn->prepare($sql);

foreach ($data as $id => $value) {
    $idParts = explode("-", $id);
    $count = $value['count'];
    $stmt->execute([$idParts[0], $idParts[1], $count, $idParts[0], $idParts[1], $count, $idParts[0], $idParts[1], $count, $idParts[0]]);
}

Здесь используется оператор CASE для обновления нужного столбца в зависимости от значения $idParts[1]. Если $idParts[1] не соответствует ни одному из условий, то значение столбца остается неизменным. Ну и не забываем про подготовленный запрос prepare чтобы избежать возможной инъекции

Comments

[Vitsliputsli]

А теперь представьте, что вы смотрите на этот код через полгода-год, или вообще другой человек смотрит, сколько времени уйдет на то, чтобы понять что тут творится? И что этот человек подумает, когда поймет, что это на самом деле просто UPDATE `list` SET $column = ? WHERE `id` = ?

[Анастасия Foxman]

Vitsliputsli, для этого придумали комментарии в коде. Человек задал вопрос, я ответила, вам обязательно своё фи тут вставлять? Напишите другой вариант, игнорируя моё решение.

[Vitsliputsli]

Анастасия Foxman, вы и на работе на комментарии при ревью так реагируете? Не воспринимайте это лично, это претензия к коду, а не к вам, только так можно улучшать код и свои навыки. Свой вариант я написал в комментариях, можете также сделать ревью для него.
Комментарии это хорошо, но лучше, когда они не нужны, и все понятно из кода.

[Михаил Смирнов]

Анастасия Foxman спасибо, идея интересная, возьму на заметку. Но соглашусь с Vitsliputsli код с такой реализацией становится менее читабельным, но всё равно спасибо.

[Ипатьев]

Vitsliputsli, если говорить объективно, то этот код идентичен тому, который написан в вопросе, просто кейс перенесен в SQL.

Если вам лично непривычно использование кейс в запросе, то это не значит, что его вообще никому не следует использовать. Ваш комментарий - это вкусовщина. Кому-то и тернарный оператор нечитабельный.

Другое дело, что эту проблему надо решать не в лоб

[Vitsliputsli]

Ипатьев, он идентичен только функционально. К функциональности вопросов нет, но у кода есть не только функциональность, есть такая штука как читаемость, и это не вкусовщина.
Представьте что вы не читали задание и не знаете что нужно сделать и посмотрите на запрос:

UPDATE `list` 
        SET count1 = CASE WHEN id = ? AND ? = 1 THEN count1 - ? ELSE count1 END,
            count2 = CASE WHEN id = ? AND ? = 2 THEN count2 - ? ELSE count2 END,
            count3 = CASE WHEN id = ? AND ? = 3 THEN count3 - ? ELSE count3 END
        WHERE id = ?

сколько времени уйдет, чтобы понять что он делает? Подскажу - бесконечность. Не зная, что конкретно подставляем вместо вопросиков, что он делает неизвестно. Давайте улучшим, добавим именованые параметры:

UPDATE `list` 
        SET count1 = CASE WHEN id = :id_part1 AND :id_part2 = 1 THEN count1 - :count ELSE count1 END,
            count2 = CASE WHEN id = :id_part1 AND :id_part2 = 2 THEN count2 - :count ELSE count2 END,
            count3 = CASE WHEN id = :id_part1 AND :id_part2 = 3 THEN count3 - :count ELSE count3 END
        WHERE id = :id_part1

Уже лучше, теперь смотря только на запрос можно понять, что он делает, не спервого взгляда, но можно.
Но, лучше ли читаемость, чем если бы мы это сделали на php? Сравним:

const COLUMNS = [
    1 => 'column1',
    2 => 'column2',
    3 => 'column3',
];
$column = COLUMNS[$id[1]];

UPDATE `list`  SET $column = $column - :count WHERE `id` = :id_part1

В этом решении только простые конструкции и все понятно с первого взгляда. На SQL, к сожалению, так не сделаешь, в нем очень часто получаются монструозные конструкции.

И это никак не вкусовщина, если вы собираетесь сопровождать проект, то лучшая читаемость необходимость.

[Ипатьев]

Vitsliputsli, ну, насчет массива соглашусь, такой вариант будет лучше
кстати, в восьмерке же можно сразу

$column = COLUMNS[$id] ?? throw new InvalidColumnException();

Ну, в любом случае тут все равно нужна отдельная таблица и нормальный запрос. Вместо вот этого всего