Пятница. Говнокод. Парсинг плейсхолдеров.

Фанат

oncle terrible
Команда форума
1. Можно избавиться от параметра "a" (и в запросе и в коде), а отталкиваться от того факта, что $value = array.
Нельзя.
1. Типов массивов может быть много.
2. Дефолтный плейсхолдер без типа однозначно может означать только строку и ничего больше.

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

2. Если указывать тип массива (т.е. не "a:ids", а "i:ids" или "?i"), то вместо $this->escapeString($value) можно будет более подходящее $this->escapeInt($value) писать.
Можно, но до сих пор никогда не требовалось.
потребуется - будем делать массив интов, а не [массив держим в уме], пишем инт.
 

riff

Новичок
Всё таки не получилось у меня донести мысль правильно. Я предложил убрать флаг "a" не для экономии, а как излишний.

1. Как сейчас (я для краткости буду писать sqlstr):
PHP:
sqlstr('SELECT * FROM tbl WHERE id IN (?a)', array(array(1,2,3)));
здесь мы указали парсеру, что в данном месте будет находиться массив (строк/чисел/неважно), и в параметрах передали массив этих данных.
Парсёр обработает шаблон, встретит флаг "?a", передаст данные в функцию createIN на обработку, и затем выдаст SELECT * FROM tbl WHERE id IN ("1","2","3").

Если мы откажемся от флага "a", а шаблон перепишем так:
PHP:
sqlstr('SELECT * FROM tbl WHERE id IN (?)', array(array(1,2,3)));
то парсёр и сам смог бы догадаться, что на месте "?" должен быть "список", т.к. он в параметрах встретит array(1,2,3).
Поэтому дополнительное указание в шаблоне "?a", по-моему, излишни.

2. (Только как дополнение к первому пункту).
Сейчас все элементы массива обрабатываются как строки "...WHERE id IN ("1", "2", "3")", даже если нам надо список чисел: "...WHERE id IN (1, 2, 3)"
это можно исправить, если указывать парсёру какого типа список мы ожидаем увидеть:
PHP:
sqlstr('SELECT * FROM tbl WHERE id IN (?i)', array(array(1,2,3))); //int
sqlstr('SELECT * FROM tbl WHERE id IN (?)', array(array(1,2,3))); //string
sqlstr('SELECT * FROM tbl WHERE id IN (?n)', array(array(1,2,3))); //number
sqlstr('SELECT * FROM tbl WHERE id IN (?s)', array(array(1,2,3))); //string
sqlstr('SELECT * FROM tbl WHERE id IN (?p)', array(array(1,2,3))); //без обработки
Правя Ваш код для реализации своей мысли, я в итоге наговнокодил свой ). Наврятли кто-то скажет "какой ты молодец", но может будет хотябы интересно посмотреть (функция самодостаточная единица):
PHP:
<?php

//t-table, f-field, s-string, i-int, n-number, p-part
function sqlstr($query, $args)
{
	$pattern = '~(\?[tfsinp]?|[tfsinp]?:[a-zA-Z_][a-zA-Z0-9_]*)~u';
	$array   = preg_split($pattern, $query, null, PREG_SPLIT_DELIM_CAPTURE);
	$out     = '';

	if (key($args) === 0)
	{
		$mode = 'numeric';

		$anum  = count($args);
		$pnum  = floor(count($array) / 2);
		if ( $pnum != $anum )
		{
			throw new Exception("Number of args ($anum) doesn't match number of placeholders ($pnum) in [$query]");
		}
	} else {
		$mode = 'named';
	}

	static $escape;
	if (!$escape) {
		$escape = array(
			't' => function($data) {
				return "`$data`";
			},
			'f' => function($data) {
				return "`$data`";
			},
			'p' => function($data) use(&$escape) {
				return is_null($data) ? 'null' : (is_array($data) ? $escape['in']($data, 'p') : $data);
			},
			's' => function($data) use(&$escape) {
				return is_null($data) ? 'null' : (is_array($data) ? $escape['in']($data, 's') : '"'.$data.'"');
			},
			'n' => function($data) use(&$escape) {
				return is_null($data) ? 'null' : (is_array($data) ? $escape['in']($data, 'n') : (float)$data);
			},
			'i' => function($data) use(&$escape) {
				return is_null($data) ? 'null' : (is_array($data) ? $escape['in']($data, 'i') : (int)$data);
			},
			'in' => function($data, $type) use(&$escape)
			{
				foreach ($data as $key=>$value)
				{
					$data[$key] = $escape[$type]($value);
				}
				return implode(',', $data);
			},
		);
	}

	foreach ($array as $i => $part)
	{
		if ( ($i % 2) == 0 )
		{
			$out .= $part;
			continue;
		}

		if ($part[0] == '?')
		{
			if ($mode === 'named')
			{
				throw new Exception("Cannot mix named and positional placeholders");
			}
			$value = array_shift($args);
			$type  = trim($part, '?');
		}
		else
		{
			if ($mode === 'numeric')
			{
				throw new Exception("Cannot mix named and positional placeholders");
			}

			list($type, $key) = explode(':', $part);

			if (array_key_exists($key, $args))
			{
				$value = $args[$key];
			}
			else {
				throw new Exception("No key found for the named placeholder [$key] in the data array");
			}
		}

		$out .= $escape[$type ? : 's']($value);
	}
	return $out;
}
пример:
PHP:
echo sqlstr('<br>НЕИМЕНОВАННЫЕ плейсхолдеры
	<br>Это название таблицы ?t
	<br>это int ?i
	<br>это number ?n
	<br>это string ?s
	<br>примеры с массивами.
	<br>этот массив с указанием типа int будет обработан как массив чисел [?i].
	<br>этот массив, без указания типа, будет обработан как массив строк [?]
	<br>это массив с указанием типа string [?s]
', array(
	'TABLE', '125', '144.45', 'TEXT',
	array(1, 2, 3, 4, '5', '6 число с ошибкой'),
	array(1, 2, 3, 4, '5', 'seven', 'nine'),
	array('one', 'two', 'five', 'six'),
));


echo sqlstr('<br>ИМЕНОВАННЫЕ плейсхолдеры
	<br>Это название таблицы t:my_table
	<br>это int i:my_int
	<br>это number n:my_number
	<br>это string s:my_string
	<br>примеры с массивами.
	<br>этот массив с указанием типа int будет обработан как массив чисел [i:int_a].
	<br>этот массив, без указания типа, будет обработан как массив строк [:def_a]
	<br>это массив с указанием типа string [s:str_a]
', array(
	'my_table'=>'TABLE',
	'my_int'=>'125',
	'my_number'=>'144.45',
	'my_string'=>'TEXT',
	'int_a'=>array(1, 2, 3, 4, '5', '6 число с ошибкой'),
	'def_a'=>array(1, 2, 3, 4, '5', 'seven', 'nine'),
	'str_a'=>array('one', 'two', 'five', 'six'),
));

echo '<br>';
echo sqlstr('SELECT * FROM ?t WHERE id IN (?i)', array('my_table', array(1,2,3)));

echo '<br>';
echo sqlstr('SELECT * FROM ?t WHERE id IN (?)', array('my_table', array(1,2,3)));

echo '<br>';
echo sqlstr('SELECT * FROM t:my_tbl WHERE id IN (i:my_arr)', array('my_tbl'=>'my_table', 'my_arr'=>array(1,2,3)));
 
Последнее редактирование:

флоппик

promotor fidei
Команда форума
Партнер клуба
то парсёр и сам смог бы догадаться, что на месте "?" должен быть "список", т.к. он в параметрах встретит array(1,2,3).
Это, как я понимаю, противоречит смыслу библиотеки — типизации передаваемых данных.
 

Фанат

oncle terrible
Команда форума
Во-первых, это не тебя не поняли, а ты. причем даже ответ не прочитал.
С этим аргументом поспорить не могу, зато появляется возможность типизировать данные в массиве.
Во-вторых, я же написал - типизовать массив совсем несложно. Вопрос - надо ли.
 

riff

Новичок
Во-первых, это не тебя не поняли, а ты.
Ну хорошо, а то я переживал, что я непонятно пишу.

Из Вашего ответа я не понял:
Нельзя. 1. Типов массивов может быть много.
Что значит "много типов массивов". Массив может быть лишь один - с данными, которые мы должны подставить в запрос.

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

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

потребуется - будем делать массив интов.
Ваш парсёр всё равно превратит их в строки в запросе.

типизовать массив совсем несложно
В Вашем коде это невозможно сделать.

Так что ответы я читал, не понял правда, но читал.
 

Breeze

goshogun
Команда форума
Партнер клуба
Ваш парсёр всё равно превратит их в строки в запросе.
если либа имеет простой механизм добавления новых плейсхолдеров, то пофиг на изначальную типизацию
 

Фанат

oncle terrible
Команда форума
Массив может быть лишь один - с данными, которые мы должны подставить в запрос.
И вот эти самые данные могут быть разных типов. Например - для массива на вставку. Или - как многие предлагают - массив идентификаторов, для селектов и традиционных инсертов. Или предлагаемый тобой массив интов.

Как тебе правильно написал флоппик, идея в том, чтобы не было никакой магии. чтобы поведение системы не менялось из-за того, какие данные пришли. Это РЕЗКО повышает предсказуемость и упрощает отладку.
А большинству писак только и надо чтобы запихнуть в одну строку побольше кода, побыстрее сдать заказчику, получить бабки, и забыть о нем. А кому-то потом придётся отлаживать, когда что-то сломается. Об этом вы не думаете. А всё - ради экономии ОДНОЙ буквы! (этот пассаж полчился резким, но относится он не к тебе персонально,а ко всем любителям write-only кода, когда ради скорости написания приносятся в жертву читабельность и отлаживаемость кода - болезнь ОЧЕНЬ многих программистов )

И всё-таки не понимаю, чем тебе так эта одна буква не угодила.

Запрос будет неправильным, если перепутаны местами параметры
Запрос будет неправильным, если данные приедут не того типа, которого мы хотим. Но парсер их схавает, потому что будет обучен есть всё подряд. Что может привести к непредсказуемому поведению.
Именно поэтому и введена строгая типизация.
Ваш парсёр всё равно превратит их в строки в запросе.
Выше Breeze уже ответил.
 
Последнее редактирование:

Breeze

goshogun
Команда форума
Партнер клуба
кстати, если уж хочется универсальности, то она должна быть в возможности легкого расширения функциональности, а не в попытке угадать что за хрень юзер подсунул.
по большому счету всего два мета-типа: группа четко определенных и строка по-умолчанию если НЕХ пришла

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

grigori

( ͡° ͜ʖ ͡°)
Команда форума
стоит разделить либу для генерации запироса и парсер: либа может ограничивать разработчика ради безопасности всего решения, а парсер все-таки ограничивать не должен,
как минимум DI для парсера в либе сделать стоит, чтобы парсер можно было переопределить

меня злит, когда я вижу в движке код вида return new CDBDriver(); и должен переписать пол-фреймворка чтобы добавить поддержку репликации
 

grigori

( ͡° ͜ʖ ͡°)
Команда форума
парсер - это код, который разбирает строку 'SELECT * FROM tbl WHERE id IN (?a)'

формирование готового запроса c нативными плейсхолдерами я бы тоже вынес в отдельный класс, чтобы можно было работать с mysqli и с pdo, подключая нужный драйвер

а либа - это код, который берет результат работы парсера, приводит пользовательские данные к нужным типам, вызывает класс, который формирует готовый нативный запрос, байндит данные и делает вызов ->exec()
 
Последнее редактирование:

riff

Новичок
В одном из последних сообщений увидел ссылку на Вашу же тему, там на хабр, там на библиотеку http://pyha.ru/go/godb/. Пользоваться ею нет никакого желания - нечто огромное. Но понравился способ именования плейсхолдеров. Если у Вас ?i и i:my_int, то там ?i и ?i:my_int. Разница мизерная, зато упрощается рег.выр., плюс позволяет добавлять свои флаги (если кому надо) и/или давать флагам осмысленные имена (?list, ?str), и вставки плейсхолдеров в коде заметнее.

Взял я ваш парсер и переделал по примеру вышесказанного. (смотрю, тестирую, но пока всё нравится)
Замене подверглись рег.выр.:
PHP:
$pattern = '~(\?(?:tbl|t|col|c|set|str|s|int|i|list_int|list|li|p)?(?::[a-zA-Z_][a-zA-Z0-9_]*)?)~';
tbl, t - название таблицы (синонимы)
col, c - название колонки (синонимы)
set - ассоциативный массив "столбец"=>"значение"
str, s - строка (синонимы)
?без указания флага тоже, что и ?s
int, i - число (синонимы)
list - список значений в виде порядкового массива
list_int, li - аналог ?list, только все элементы обрабатываются как числа (синонимы)
p - часть вставляемая без обработки


ну и в секции "foreach ($array as $i => $part)" тоже надо чуть поправить.

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

Ничего, если я размещу здесь ссылку на свой вариант, чтобы было в одном месте?
Изменённый вариант встроил в свой класс для работы с DB и выложил код сюда http://pastebin.com/gppkjMDM.
Пример (взят из ответа в другой теме). Теперь, чтобы сделать, например, выборку мне достаточно написать:
PHP:
$row = DB::query('SELECT ... WHERE name = ?', array('Вася'))->get();
или $row = DB::query('SELECT ... WHERE id = ?i', array(55))->get();
или $rows = DB::query('SELECT ... WHERE id > ?i:min AND id < ?i:max',
          array('min'=>3, 'max'=>10))->getAll();
или $rows = DB::query('
           SELECT * FROM users
           WHERE user_start_time <= ?i:time AND user_end_time > ?i:time
           LIMIT ?i:limit', array('time'=>1348372943, 'limit'=>5)->getAll();
 
Последнее редактирование:

Фанат

oncle terrible
Команда форума
Но понравился способ именования плейсхолдеров.
Хорошая идея, мне нравится. Спасибо. Я как раз все думал над вопросом именования.

Регулярку, в принципе, все равно услжнять придется - желательно, чтобы внутри кавычек она не срабатывала. всех трех типов. Это весьма редкий случай, но всё же.
 

riff

Новичок
Пока тестировал, столкнулся с:
PHP:
sqlstr('
    INSERT INTO ?tbl SET ?set
', array(
    'my_table',
    array(
        'field1'=>123,
        'field2'=>'NOW()', //<-----
        'field3'=>'my text',
    )
));
По факту все данные должны обработаться как строки, но нам нужна функция. Что делать с этим не понятно.
Можно придумать типа "?set(ips)", т.е. указывать флаги для полей, но я пока сделал так:
PHP:
sqlstr('
    INSERT INTO ?tbl SET ?set
', array(
    'my_table',
    array(
        'field1'=>123,
        'field2'=>array('p' =>'NOW()'), //<-----
        'field3'=>'my text',
    )
));
И соответственно в парсере добавить обработчик.
 
Сверху