Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SC_Query setOrder() と setLimit() の有効範囲を揃える #1116

Open
seasoftjapan opened this issue Jan 5, 2025 · 2 comments · May be fixed by #1134
Open

SC_Query setOrder() と setLimit() の有効範囲を揃える #1116

seasoftjapan opened this issue Jan 5, 2025 · 2 comments · May be fixed by #1134

Comments

@seasoftjapan
Copy link
Contributor

seasoftjapan commented Jan 5, 2025

#1097 から派生。


$objQuery = SC_Query_Ex::getSingletonInstance();
$objQuery->begin();
$objQuery->delete('dtb_bkup');
$objQuery->insert('dtb_bkup', ['bkup_name' => 'example1']);
$objQuery->insert('dtb_bkup', ['bkup_name' => 'example2']);
$objQuery->insert('dtb_bkup', ['bkup_name' => 'example3']);
echo implode(', ', $objQuery->getCol('bkup_name', 'dtb_bkup'))."\n";
$objQuery->setLimit(1);
$objQuery->setOrder('bkup_name DESC');
echo implode(', ', $objQuery->getCol('bkup_name', 'dtb_bkup'))."\n";
echo implode(', ', $objQuery->getCol('bkup_name', 'dtb_bkup'))."\n";
$objQuery->rollback();
  • 出力の分かりやすさから getCol() を使っているが、select() なども同様。

実行結果

example1, example2, example3
example3
example3, example2, example1
  • getCol() の3回目の戻り値は、一般的には1回目か2回目と同じ内容を期待すると思う。しかし、setOrder() は有効なまま setLimit() が無効となる。

setLimit() のみ無効となるのは、(狙った挙動というより) MDB2 の処理に依存していそう。

setOrder() を使った影響を回避するため $objQuery = SC_Query_Ex::getSingletonInstance(); を再実行する実装は散見する反面、setOrder() が維持されるのを期待した実装はほぼ1皆無という認識。よって、setOrder() も一度限りで無効化する方向を軸に考えている。それにより、同一インスタンスで setOrder() など → select() など → delete() の流れで実行するとコケる問題 も回避できると見込んでいる。

Footnotes

  1. EC-CUBE 本体ソースを確認すると、唯一 SC_Product::getListByProductIds() に該当が見つかりました。はい、私が実装しました... 影響しないよう調整します。

@seasoftjapan
Copy link
Contributor Author

seasoftjapan commented Jan 5, 2025

もしも、SC_Product::getListByProductIds() 以外の影響や、むしろ setLimit() を setOrder() に合わせたいなどの意見がありましたら、早めにお知らせいただけますと幸いです。

@bbkids
Copy link
Contributor

bbkids commented Jan 5, 2025

setOrder() と setLimit() いずれも有効範囲が同じであるべきだと思います。
私も select() の時点で、setLimit()と同様にsetOrder() の効力を失効させるので良いと思います。

@nanasess nanasess modified the milestones: 2.18(仮), 2.x Jan 9, 2025
@seasoftjapan seasoftjapan changed the title SC_Query setOrder() と setLimit() で有効範囲が異なる SC_Query setOrder() と setLimit() の有効範囲を揃える Jan 9, 2025
seasoftjapan added a commit that referenced this issue Jan 9, 2025
- getSql() LIMIT, OFFSET に対応。処理は SC_DB_DBFactory::addLimitOffset() に委ねる。
- getSql() 末尾で、句に関わる設定をリセットする。
- setLimitOffset()、setLimit()、setOffset() いずれも、MDB2 のラッパーではなく、getSql() に依存させる。
seasoftjapan added a commit that referenced this issue Jan 10, 2025
- getSql() LIMIT, OFFSET に対応。処理は SC_DB_DBFactory::addLimitOffset() に委ねる。
- getSql() 末尾で、句に関わる設定をリセットする。
- setLimitOffset()、setLimit()、setOffset() いずれも、MDB2 のラッパーではなく、getSql() に依存させる。
seasoftjapan added a commit that referenced this issue Jan 10, 2025
- 共通部分 (SC_*) テスト追加
- 先日追加した SC_DB_DBFactory*Test はメンテが煩雑なので抽象化した。
seasoftjapan added a commit that referenced this issue Jan 10, 2025
- 共通部分 (SC_*) テスト追加
- 先日追加した SC_DB_DBFactory*Test はメンテが煩雑なので抽象化した。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants