mysql_connect()の5番目の引数client_flagsにMYSQL_CLIENT_FOUND_ROWSを追加？

[K. Ono]

onokazuです。

minahitoさんの記事（http://d.hatena.ne.jp/minahito_carp/20090317#1237251786）
でネタ募集中とのことでしたので、早速こちらに投稿させていただきます（どこに
投稿するか迷いましたが）。

まさかclass/database下に修正が入るとは思っていなかったので、これまでリクエスト
しようとは思いませんでしたが、今回それが行われるということで2.1.7で下記もご検討
いただければと。。

mysqlに関してですが、mysql_connect()の５番目の引数client_flagsに
MYSQL_CLIENT_FOUND_ROWSを渡すように変更していただけない
かなと思います。

MYSQL_CLIENT_FOUND_ROWSはphpではdefineされて
いないので、下記のようなものをmysqldatabase.phpの上の方に
追加する必要があると思います。

if (!defined('MYSQL_CLIENT_FOUND_ROWS')) {
define('MYSQL_CLIENT_FOUND_ROWS', 2);
}

そして、connect()メソッドを下記のように

function connect($selectdb = true)
{
if (XOOPS_DB_PCONNECT == 1) {
$this->conn = @mysql_pconnect(XOOPS_DB_HOST, XOOPS_DB_USER,
XOOPS_DB_PASS, false, MYSQL_CLIENT_FOUND_ROWS);
} else {
$this->conn = @mysql_connect(XOOPS_DB_HOST, XOOPS_DB_USER,
XOOPS_DB_PASS, false, MYSQL_CLIENT_FOUND_ROWS);
}

...省略
}

なぜこのような変更が必要かと言いますと、MYSQL_CLIENT_FOUND_ROWSなしだと
下記のようなクエリを実行した場合、mysql_affected_rows()は必ず0を返してしまう
からです。

UPDATE テーブル SET id = 1 WHERE id = 1;

マッチした行があっても、内容に変更がない場合には0を返すとのことです。

pg_affected_rows()やsqlite_changes()では上のクエリは1を返すとのこと（pg_affected_rows()未確認）
なので、mysqlもそれに合わせておかないと場合によっては不具合が発生するのでは
と思います。

preloadで対応したりしてみましたが、やはりこちらを書き直さない限りにはうまくいかない
模様です。

ただ、これまでこの仕様に依存してきたモジュールがあると問題ですが、どうなんでしょう？

[minahito]

minahitoです。

こんなパラメータがあったとは...(汗
いつも貴重な情報ありがとうございます。

> pg_affected_rows()やsqlite_changes()では上のクエリは1を返すとのこと（pg_affected_rows()未確認）
> なので、mysqlもそれに合わせておかないと場合によっては不具合が発生するのでは
> と思います。

&&

> ただ、これまでこの仕様に依存してきたモジュールがあると問題ですが、どうなんでしょう？

つまり、

- 修正しないと場合によっては不具合が発生するのでは!!
- でも修正すると場合によっては不具合が発生するのでは... orz

ということですね...ありがちですね;;
もう、これはもうどっちか選ぶしか無いですね (^^;

個人的には修正してしまいたいですが、ただ、 XOOPS2 的にみれば、

事実上 MySQL しかサポートしていない XOOPS で、 Postgre や sqlite と同等の
反応を求めてコードを書いてしまう人がいればその人が XOOPS の Requirement を
無視しているだけだと言えますし、

従来の動作に依存しているコードを書いている人がいるとすれば、
XOOPS の提供する API の反応に合わせて、よくテストしていると言えると思います。

ここを修正するということは、その価値観を引っくり返すということになりますから、
そこはちょっと申し訳ないですね。。。

ただ、Legacy 以外の BASE は間違いなくマルチDBになりますから、
実際に周りの DB に合わせておかないと不具合の出るプログラムが現状に登場するか
どうかではなく、 XCL の基準として予め合わせておくことには意味があると思います。

とはいえ、ホント、実害の種類と数次第ですね...

海外のポピュラーな XOOPS2 系モジュールで依存コードがあったら、
直してくれとも言えないし、都度パッチあてが必要になっちゃったら、
やっぱユーザー視点からいけば改悪って言われちゃうでしょうし。

かなりマニアックな話なので、
もし知っている人がいれば情報を寄せていただきたいのですが、
最終的には RC か別パッチで稼働の情報を集めてみるのが一番いいかなぁ......

（別パッチとして出して、有志のテスターに調べてもらって、
　問題が報告されなかったら 2.1.7 の次のバージョンでマージ？）

2009/03/17 17:38 K. Ono <ono...@gmail.com>:

--
minahito (mina...@gmail.com)

[K. Ono]

onokazuです。

そうですね。実際のところ、XoopsMySQLDatabase::getAffectedRows()
を使用しているモジュールってどの程度あるのでしょうね。

こちらではXoopsMySQLDatabaseは使用していないのですが、mysql_connect()で
確立されたコネクションは再利用されるので、別のDBライブラリを使用した場合も
MYSQL_CLIENT_FOUND_ROWSが指定されていないのと同じ挙動になってしまいます。

そこで更に調べてみるとmysql_info()を使用すれば直前のクエリでマッチした行の数を
取得できるみたいです。
http://php.net/manual/en/function.mysql-info.php

なので、XCLでの対応がどうしても無理ということであれば、こちらのDBライブラリ
の方でmysql_info()を使って対応しようかなとも思います。パフォーマンス的に
どうなってしまうのか不明ですが。。

[minahito]

minahitoです。
お疲れ様です。

> なので、XCLでの対応がどうしても無理ということであれば、こちらのDBライブラリ
> の方でmysql_info()を使って対応しようかなとも思います。

コード検索とかでざっと調べた限り見つからなかったので、
XCL 側で修正しちゃいます。

一週間ずれちゃいましたが、これで今回の内容は確定でしょうか。

2009/03/18 20:39 K. Ono <ono...@gmail.com>:

--
minahito (mina...@gmail.com)

[minahito]

minahito です。
お疲れさまです。

今件、コミットしておきました。
貴重なネタありがとうございました。
（言われなかったら絶対気づかなかった）

RC のタイミングで構いませんので、一度ご確認願います。 m(__)m

2009/03/20 15:37 minahito <mina...@gmail.com>:

--
minahito (mina...@gmail.com)