Add write support: INSERT/UPDATE/DELETE/DDL/TCL execution#3
Conversation
Extend SQLiteView from a read-only viewer to a full SQL client that supports INSERT, UPDATE, DELETE, CREATE, DROP, BEGIN/COMMIT/ROLLBACK, and other write operations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、SQLiteViewを読み取り専用のビューアから、完全なSQLクライアントへと進化させます。これにより、ユーザーはデータベースの構造やデータを直接変更できるようになり、アプリケーションの有用性が大幅に向上します。変更はデータベース層、UI層、シンタックスハイライト、およびテストにわたって行われ、安全かつ効率的な書き込み操作を保証するための機能が追加されています。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
このプルリクエストは、SQLiteViewに書き込み機能を追加するための重要な変更を含んでいます。データベース層での書き込み許可、UIでの書き込み結果の表示や破壊的クエリの警告、SQLハイライトのキーワード追加など、多岐にわたる改善が行われています。全体として、機能追加は計画通りに進んでいるように見えます。いくつか改善点を提案します。まず、database.pyの破壊的クエリ検出ロジックには、コメントによってバイパスされる可能性がある脆弱性が存在します。次に、mainwindow.pyでは、カプセル化の原則に反するプライベートメソッドの呼び出しや、DML後のUI更新ロジックの堅牢性に改善の余地があります。最後に、ドキュメント内のコマンド例も、より汎用的にすると良いでしょう。これらの点を修正することで、より安全で保守性の高いコードになるでしょう。
src/sqliteviewer/database.py
Outdated
| upper = sql.upper() | ||
| if "WHERE" not in upper: | ||
| return True, "DELETE without WHERE will remove all rows." |
There was a problem hiding this comment.
DELETE 文の WHERE 句の有無をチェックするロジックが、SQLコメント内に WHERE キーワードが含まれている場合に正しく機能しません。これにより、意図せずテーブルの全行が削除される危険な操作を許可してしまう可能性があります。
例えば、DELETE FROM users -- WHERE id = 1 のようなクエリは、コメント内に WHERE があるため安全と誤判定されますが、実際には全行を削除します。
この問題を解決するには、WHERE の存在をチェックする前に、SQLクエリからコメントを削除する処理を追加することを強く推奨します。正規表現や簡単なパーサーを実装することで、より堅牢なチェックが可能になります。
PLAN.md
Outdated
|
|
||
| ```bash | ||
| # ユニットテスト実行 | ||
| cd /home/yugosasaki/code/SQLiteView && uv run python -m pytest tests/ -v |
There was a problem hiding this comment.
コマンド例に、ハードコードされたユーザー固有の絶対パス（/home/yugosasaki/code/SQLiteView）が含まれています。これにより、他の開発者がコマンドを直接コピー＆ペーストして使用することができません。プロジェクトのルートディレクトリからコマンドを実行するように指示するか、cd部分を削除してコマンドをシンプルにすることを推奨します。
| cd /home/yugosasaki/code/SQLiteView && uv run python -m pytest tests/ -v | |
| uv run python -m pytest tests/ -v |
src/sqliteviewer/mainwindow.py
Outdated
| def _refresh_after_write(self, sql: str) -> None: | ||
| """Refresh UI panels after a write operation.""" | ||
|
|
||
| query_type = self.database_service._classify_query(sql) |
There was a problem hiding this comment.
DatabaseService のプライベートメソッドである _classify_query （アンダースコアで始まる）を MainWindow から直接呼び出しています。これはカプセル化の原則に違反し、DatabaseService の内部実装の変更が MainWindow に影響を与える可能性があるため、コードの保守性を低下させます。
_classify_query が DatabaseService の公開APIの一部として意図されている場合は、メソッド名を classify_query に変更して公開メソッドにすることを検討してください。
| query_type = self.database_service._classify_query(sql) | |
| query_type = self.database_service.classify_query(sql) |
| selected_items = self.table_list.selectedItems() | ||
| if selected_items: | ||
| table_name = selected_items[0].text() | ||
| if table_name.lower() in sql.lower(): |
There was a problem hiding this comment.
DML操作後にデータプレビューを更新する現在のロジックは、単純な部分文字列チェック（table_name.lower() in sql.lower()）に依存しています。このアプローチは堅牢ではなく、以下のような場合に意図しない動作を引き起こす可能性があります。
- テーブルがエイリアスで参照されている場合に更新されない。
- SQLクエリ内の文字列リテラルやコメントにテーブル名が含まれている場合に誤って更新される。
- 別のテーブルへの操作にもかかわらず、選択中のテーブル名がクエリ内に偶然含まれている場合に誤って更新される。
より信頼性の高いアプローチとして、SQLを簡易的に解析して、操作対象のテーブル名をより正確に特定することを検討してください。例えば、正規表現を用いて INSERT / UPDATE / DELETE 文からテーブル名を抽出する方法が考えられます。
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b064d2d16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if result.is_write_operation: | ||
| self.query_result_view.setModel(None) |
There was a problem hiding this comment.
When a write statement succeeds, _run_query clears only the table view model but leaves self.query_result untouched. If the user previously ran a SELECT, then runs INSERT/UPDATE/DELETE, the UI shows no result rows yet Export Results still exports the old SELECT dataset from memory, which can silently produce incorrect exports.
Useful? React with 👍 / 👎.
src/sqliteviewer/database.py
Outdated
| upper = sql.upper() | ||
| if "WHERE" not in upper: |
There was a problem hiding this comment.
The destructive-query guard decides whether a DELETE is safe by checking if the raw SQL text contains the substring WHERE, so comments or string literals can bypass the warning (for example, DELETE FROM users /* WHERE id=1 */ still deletes all rows but will not trigger confirmation). This weakens the new safety prompt and can lead to accidental data loss for users relying on it.
Useful? React with 👍 / 👎.
- CIエラー修正: build_deb.sh のパッケージ名を sqliteviewer→sqliteview、 バージョンを 0.1.0→0.2.1 に更新し、control/実行ファイルにも変数を使用 - セキュリティ修正: DELETE の WHERE チェック前に SQL コメントを除去する _strip_sql_comments() を追加し、コメントによるバイパスを防止 - バグ修正: write 操作後に self.query_result を None にリセットし、 古い SELECT 結果が Export で誤出力されないよう修正 - カプセル化改善: _classify_query を classify_query に公開化し、 MainWindow からの呼び出しを適切な公開 API 経由に変更 - ドキュメント改善: PLAN.md のハードコードパスを削除 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
database.py:_assert_read_only()を削除し、INSERT/UPDATE/DELETE/CREATE/DROP/BEGIN等すべての SQL 文を実行可能にした。QueryResultにis_write_operationとaffected_rowsを追加。_classify_query()とis_destructive_query()を追加mainwindow.py: 書き込み結果の表示（N row(s) affected）、破壊的操作の確認ダイアログ、DDL/DML 後の自動リフレッシュ（_refresh_after_write()）、テーブル選択の復元を実装sql_highlighter.py: INSERT/UPDATE/DELETE/CREATE/DROP/BEGIN/COMMIT/ROLLBACK 等の書き込み系キーワード約50語を追加tests/test_database.py: 書き込み系テスト（INSERT/DELETE/DDL/トランザクション/クエリ分類/破壊的操作検出）を追加。10テスト全 passTest plan
uv run python -m pytest tests/ -v— 10テスト全 pass を確認INSERT INTO ... VALUES (...)が正常に実行されることUPDATE ... SET ...実行後、Data Preview が自動更新されることDROP TABLE ...実行時に確認ダイアログが表示されることDELETE FROM ... (WHERE なし)実行時に確認ダイアログが表示されることCREATE TABLE ...後、テーブルリストに新テーブルが追加されることBEGIN→ 変更 →ROLLBACKでロールバックされること🤖 Generated with Claude Code