Skip to content

Conversation

AlexeyDsov
Copy link
Member

Так сложилось, что текущие способы работы с базами данных требуют подключения к базе данных, т.к. на основе него определяют как корректно выполнять ту или иную операцию (например, эскейпинг строки). Если в методе не задается соединение к базе, то он использует первое попавшееся. Это касается всех баз - sqlite, postgres, mysql. По этой причине я сделал все диалекты не синглтонами, их пораждает наследник DB (PgSQL, MySQL и т.д.) по требованию getDialect и задаёт себя диалекту, что бы он имел возможность использовать коннект к базе что бы часть методов таких как quoteValue, ставшие не статическими, могли указывать какое соединение к базе данных они используют.

Внешне для обычного использования API измениться не должно было. В коде кроме метода DB::getDialect и различных тестов никто не вызывал статически методы *Dialect::me и другие ставшие не статическими.

Сами тесты были переписаны таким образом, что они получают базу и диалект из тестового конфига.

@AlexeyDsov AlexeyDsov mentioned this pull request Sep 16, 2012
@ewgRa
Copy link
Contributor

ewgRa commented Sep 19, 2012

А почему бы просто не добавить в методы, которые требуют подключение к БД дополнительный аргумент?

Собственно кроме ескейпа есть еще методы которым нужно подключение к БД?

@AlexeyDsov
Copy link
Member Author

@ewgRa А кто будет этот аргумент передавать?

@AlexeyDsov
Copy link
Member Author

Подумал и дописал: По коду как таковому диалект, не смотря на то что он синглтон везде используется будто бы он не синглтон. Тут в diff исправления по большему счету это лишь: 1) DB наследники, типа PgSQL 2) Сами диалекты 3) и тесты.

@ewgRa
Copy link
Contributor

ewgRa commented Sep 19, 2012

@AlexeyDsov кто будет escape дергать, тот и будет передавать

@AlexeyDsov
Copy link
Member Author

В таком случае нужно поправить все Expression и другие logic'и и по всей цепочке вложенности всяких Expression,
DBvalue и т.д. В методе toDialectString у них нужно передавать зачем-то этот самый link. О нем они все совсем знать не должны, не их это дело знать о базе.

Не лучше ли что б под каждое подключение создавался свой личный диалект вместо одного общего синглтона?
Да и почему вообще они должны быть синглтонами и что мешает от этого избавиться?

@dovg
Copy link
Member

dovg commented Sep 19, 2012

Меня смущает, что линк нужен только mysql, но при этом присутствует во всех.
Для того же Imaginary само понятие линка есть ересь ИМХО.

То, что диалекты стали не singleton, заставит нас переписать много кода, но это частности. )

@AlexeyDsov
Copy link
Member Author

@dovg неправда ваша, линк нужен почти во всех диалектах, за исключением Imaginary.
А какого рода код вас заставляет это изменение переписать? Я надеялся что изменений в проектах это потребует минимум. Только если где-то вызывается *Dialect::me

@ewgRa
Copy link
Contributor

ewgRa commented Sep 19, 2012

@AlexeyDsov соглашусь.

@AlexeyDsov
Copy link
Member Author

Еще по эскейпингу кину ссылочек :)

  • http://php.net/manual/en/function.pg-escape-string.php - не обязательный аргумент $link, но при этом если он не задан, то функция выбирает линк по своему усмотрению (т.е. последний активный коннект, если он есть). А если, например, используется кэширование и соединение с базой еще не было установленно, то оно вообще может эскейпить другим способом нежели когда есть соединение.
  • http://php.net/manual/en/function.mysql-escape-string.php - тут соединения нет, но советуют не использовать эту функцию
  • http://php.net/manual/en/mysqli.real-escape-string.php - предлагается как альтернатива всем другим эскейпингам mysql, с линком все тоже самое как и в pg_escape_string
  • http://php.net/manual/en/pdo.quote.php - в случае PDO метод тупо вызывается из самого "линка", т.е. в этого случае самого PDO.

Т.е. почти все функции либо желают иметь линк, либо требуют, иначе они могут эскейпить непредсказуемо.

@dovg
Copy link
Member

dovg commented Sep 19, 2012

Извините, был напуган.

AlexeyDsov added a commit that referenced this pull request Sep 29, 2012
Dialects not Singletons now. Some methods now unstatic and they use DB for correct quoting
@AlexeyDsov AlexeyDsov merged commit cbafefa into onPHP:master Sep 29, 2012
@AlexeyDsov
Copy link
Member Author

Замержил. На страничке 1.0 to 1.1 написал что нужно поменять, что б ничего не сломалось:

При миграции в коде нужно найти cтроки *Dialect::me( и заменить на код вида DBPool::getByDao(SomeMyClass::dao())->getDialect()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants