Покритикуйте подход и реализацию?

Вурдалак

Продвинутый новичок
Фанат, если строго, то нет, потому что если отойти от запросов, то возможна ситуация: «???a» — тут знак вопроса и плейсхолдер, но твой код не найдёт тут плейсхолдера. Но в случае с SQL-запросами я как-то не могу представить себе ситуацию, когда такое вылезет, поэтому можно и забить, конечно.
 

Фанат

oncle terrible
Команда форума
Короче, я туплю не по дням а по часам.
Добавил одну строчку в код, чтобы он стал рабочим.

Ситуация с «???a», действительно, в запросе невозможна.
 

Фанат

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

Фанат

oncle terrible
Команда форума
Короче, в итоге всё вылилось в такой вот говнокод, ровно половину которого занимает экранирование знака вопроса :)
про исключения помню, они на очереди
PHP:
private function prepareQuery($args)
{
	$query = array_shift($args);
	preg_match_all('~(\?[a-z?])~',$query,$m,PREG_OFFSET_CAPTURE);
	$pholders = $m[1];
	$count = 0;
	foreach ($pholders as $i => $p)
	{
		if ($p[0] != '??')
		{
			 $count++;
		}
	}
	if ( $count != count($args) )
	{
		trigger_error("Number of args doesn't match number of placeholders");
		return FALSE;
	}
	$shift  = 0;
	$qmarks = 0;
	foreach ($pholders as $i => $p)
	{
		$pholder = $p[0];
		$offset  = $p[1] + $shift;
		if ($pholder != '??')
		{
			$value   = $args[$i-$qmarks];
		}
		switch ($pholder)
		{
			case '?n':
				$value = $this->escapeIdent($value);
				break;
			case '?s':
				$value = $this->escapeString($value);
				break;
			case '?i':
				$value = $this->escapeInt($value);
				break;
			case '?a':
				$value = $this->createIN($value);
				break;
			case '?u':
				$value = $this->createSET($value);
				break;
			case '??':
				$value = '?';
				$qmarks++;
				break;
			default:
				trigger_error("Unknown placeholder type $pholder");
				return FALSE;
		}
		$query = substr_replace($query,$value,$offset,2);
		$shift+= strlen($value) - strlen($pholder);
	}
	return $query;
}
 

Духовность™

Продвинутый новичок
а у меня так

PHP:
    /**
     * Выполняет SQL-запрос.
     *
     * Результатом будет либо возвращённое булево значение,
     * либо объект класса Krugozor_DB_MysqlStatement.
     *
     * Метод принимает обязательный параметр - SQL-запрос и, в случае наличия,
     * любое количество аргументов - значения placeholder.
     * Метод использует технологию placeholders - для вставки данных в строку
     * SQL-запроса используются специальные маркеры, а сами данные передаются "позже".
     * Данные, прошедшие через систему placeholders,
     * экранируются специальными функциями экранирования,
     * в зависимости от типа сравнения (см. далее). Т.е. вам нет
     * необходимости заключать переменные в функции
     * экранирования типа {@link mysql_escape_string()}.
     *
     * Существует несколько видов маркеров:
     * ?s и ? - маркер типа string, данные экранируются как строка
     * ?S - маркер типа string, данные экранируются как строка для подстановки в LIKE
     * ?i - маркер типа int, данные приводятся к типу int
     * ?a - маркер хеш-массива.
     *
     * @param string строка SQL-запроса
     * @param mixed подставляемые значения для placeholder-ов.
     * @return mixed Возвращает либо bool, либо объект класса Krugozor_DB_MysqlStatement.
     */
    public function query()
    {
        $this->connect();

        // Если аргументов в функцию, не переданно, возвращаем FALSE.
        if (!$c = func_num_args()) {
            return false;
        }

        $arg_list = func_get_args();

        // Ссылка на запрос.
        $q = &$arg_list[0];
        // Экранируем символ, т.к. этого требует sprintf
        $q = str_replace('%', '%%', $q);

        // Массмв, в который будем записывать placeholder's
        // в порядке появления их в строке.
        $ph = array();

        // Новый запрос, после обработки нижестоящим кодом.
        $nq = '';

        $strlen = strlen($q);

        // Парсим запрос, по символам.
        for ($i=0; $i<$strlen; $i++)
        {
            // Обнаруживаем метку-заполнитель `?`
            if ($q[$i] === '?')
            {
                if ($q[$i+1] === '?')
                {
                    $nq .= $q[$i];
                }
                else if ($q[$i+1] === 's' && isset($q[$i+1]) && $q[$i+1] !== '?')
                {
                    $nq .= '%s';
                    $ph[] = '?s';
                }
                else if ($q[$i+1] === 'S' && isset($q[$i+1]) && $q[$i+1] !== '?')
                {
                    $nq .= '%s';
                    $ph[] = '?S';
                }
                else if ($q[$i+1] === 'i' && isset($q[$i+1]) && $q[$i+1] !== '?')
                {
                    $nq .= '%s';
                    $ph[] = '?i';
                }
                else if ($q[$i+1] === 'a' && isset($q[$i+1]) && $q[$i+1] !== '?')
                {
                    $nq .= '?a';
                    $ph[] = '?a';
                }
                else if ($q[$i+1] === 'n' && isset($q[$i+1]) && $q[$i+1] !== '?')
                {
                    $nq .= '%s';
                    $ph[] = '?n';
                }

                $i++;
            }
            // это обычный символ.
            else {
                $nq .= $q[$i];
            }
        }

        // SQL-запрос
        $q = $nq; // тут в действительности присваиваем значение элементу $arg_list[0]

        // Смотрим, объявленны ли маркеры массива
        while (($index = array_search('?a', $ph)) !== FALSE)
        {
            $index += 1;

            // Создаем строку SQL-запроса и добавляем в аргументы значения из массива
            if (is_array($arg_list[$index]))
            {
                $array_keys = array_keys($arg_list[$index]);
                $array_values = array_values($arg_list[$index]);
                unset($arg_list[$index]);
                $arg_list = self::array_push_before($arg_list, $array_values, $index);
                $sql_array = '';

                foreach ($array_keys as $value)
                {
                    $sql_array .= " `$value` = \"%s\",";
                    $pls[] = '?s';
                }

                $sql_array = trim($sql_array, ',');

                unset($ph[$index-1]);
                $ph = self::array_push_before($ph, $pls, $index-1);
                $q = self::str_replace_once('?a', $sql_array, $q);
            }
        }

        $w = 0;
        foreach ($arg_list as $k => $v)
        {
            // k = 0 - это всегда SQL-запрос, поэтому эскейпить его не нужно!
            if ($k === 0) {
                continue;
            }

            // Если значение пусто, то никаких escape не делаем.
            if ($v === '') {
                $w++; // добавил 25 мая
                continue;
            }

            if ($ph[$w] === '?s') {
                $arg_list[$k] = mysql_real_escape_string($v, $this->lnk);
            }
            else if ($ph[$w] === '?S') {
                $arg_list[$k] = $this->escape_like($v);
            }
            else if ($ph[$w] === '?i') {
                $arg_list[$k] = filter_var($v, FILTER_SANITIZE_NUMBER_INT);
            }
            else if ($ph[$w] === '?n')
            {
                if ($v !== null)
                {
                    throw new Exception('Попытка записать NULL в базу при значении '.print_r($v, true));
                }

                $arg_list[$k] = 'NULL';
            }
            $w++;
        }

        // Формируем строку SQL-запроса.
        $this->query = @call_user_func_array('sprintf', $arg_list);

        if (FALSE === $this->query)
        {
            $err = error_get_last();
            throw new Krugozor_Db_Mysql_Exception(
                'Ошибка парсера sprintf: '. $err['message'], '', print_r($arg_list, true)
            );
        }

        $this->result = mysql_query($this->query, $this->lnk);

        self::$queries[] = $this->query;

        if (!$this->result)
        {
            throw new Krugozor_Db_Mysql_Exception('Ошибка в запросе: ' . mysql_error());
        }

        if (is_resource($this->result))
        {
            return new Krugozor_Db_Mysql_Statement($this->result);
        }

        return $this->result;
    }
 

Вурдалак

Продвинутый новичок
Фанат, была бы последняя версия PHP, то можно было бы всё красиво через preg_replace_callback + closure сделать. Впрочем, можно и сейчас switch в отдельный метод пихнуть, массив значений пихнуть в SplQueue и передавать как свойство объект в callback со switch'ем, и при выборке из очереди выбросится исключение, если значение не хватает. Ещё в конце проверить пуста ли очередь из значений.

Духовность™, вместо префикса «Krugozor_» тебе достаточно было везде добавить
PHP:
namespace Krugozor;
но ты же не ищешь лёгких путей.
 

Sad Spirit

мизантроп (Старожил PHPClub)
Команда форума
Короче, в итоге всё вылилось в такой вот говнокод, ровно половину которого занимает экранирование знака вопроса :)
...но и этого всё равно недостаточно:
PHP:
echo prepareQuery(array("select 'а почему оно так странно работает???'"));
 

Фанат

oncle terrible
Команда форума
...но и этого всё равно недостаточно
О, спасибо. Добавил проверку на случай, если вопросы есть, а плейсхолдеров нету.
Вопросы, кстати, надо экранировать, так что правильно будет
PHP:
echo prepareQuery(array("select 'а почему оно так странно работает??????'"));
 

Вурдалак

Продвинутый новичок
Если считать, что знаки вопроса экранировать придётся только в строковых литералах, то можно так:
PHP:
$pattern = '/"(?:\\\\.|[^"])*+"|\'(?:\\\\.|[^\'])*+\'|\?[a-z]/';
preg_match_all() или preg_split(), там есть свои плюсы и минусы.

Правда для каждой СУБД свой паттерн будет, т.к. в MySQL символы экранируются с помощью бэкслэша, а в каком-нибудь MS SQL — удваиванием символа ограничителя.
 

Sad Spirit

мизантроп (Старожил PHPClub)
Команда форума
О, спасибо. Добавил проверку на случай, если вопросы есть, а плейсхолдеров нету.
Вопросы, кстати, надо экранировать, так что правильно будет
PHP:
echo prepareQuery(array("select 'а почему оно так странно работает??????'"));
Мне это правильным не кажется, честно говоря. У тебя же подстановки внутрь строковых констант нормально обрабатываться не будут, зачем там тогда экранировать вопросики?
 

Фанат

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

Вурдалак

Продвинутый новичок
Фанат, ну как раз этим выражением ты достанешь строковые литералы и плэйсхолдеры. Там в default добавить if и всё. Строковой литерал в $value прямо и пихать, и он не будет обрабатываться.
 

Фанат

oncle terrible
Команда форума
Чтобы не пытаться делать в них подстановки, их как раз тогда и не надо доставать вообще.
 

Вурдалак

Продвинутый новичок
Фанат, ну блин, чё ты тупишь...

PHP:
function prepareQuery($args)
{
    $query = array_shift($args);
    preg_match_all('/"(?:\\\\.|[^"])*+"|\'(?:\\\\.|[^\'])*+\'|\?[a-z]/', $query, $m, PREG_OFFSET_CAPTURE);

    // ...

    foreach ($pholders as $i => $p)
    {
        // ...

        switch ($pholder)
        {
            // ...

            default:

            if( $pholder[0] === '?' )
            {
                throw new Exception('Unknown placeholder type ' . $pholder);
            }
            else
            {
                $value = $pholder; // литерал
            }
        }

        $query = substr_replace($query,$value,$offset, strlen($pholder));

        // ...
    }
}
 

MiksIr

miksir@home:~$
Ну вот как всегда, начали за здравие про плейсхолдеры, а закончат синтаксическим анализатором запроса. Это, кстати, в ответ на многие вопросы про то "а зачем нужен sql конструктор" - там эта проблема решена изначально, а тут придется разбирать текст запроса на куски, что бы нигде не ошибиться.
 
Сверху