Hacktoberfest 2026:メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

Generate sql commands using `psycopg.sql`

オープン
#134 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
30/100
issue の種類
リファクタリング
明瞭さ
おおむね明確
活発さ
停滞
技術スタック
postgresql, python
領域
cli, databases

調査の方向性

まず pgspecial/dbcommands.py、特に describe_one_table_details を調査し、コマンド実装内にあるその他の SQL 文字列の組み立て箇所を洗い出します。PostgreSQL のバージョン分岐とコマンド実行がどのようにカバーされているかを確認します。関連する生成コマンドが psycopg.sql を安全に使用し、サポートされているコマンドの動作を維持しながら、データベースの各バージョンにわたって十分なカバレッジを備えていれば完了です。

索引モデルが issue の本文から書いたものです。

説明

Hello,

my eye stumbled on #133 and it made me notice that commands are generated using string composition. This has a series of problems: the missing quotes one, but, for instance, this command wouldn't handle a schema or sequence name containing a double quote.

If you think it's ok, I can propose a MR to replace all the string composition using the psycopg.sql objects, which are designed to overcome this problem, or can assist someone to prepare one (my times might be long, I'm kinda busy at the moment)

The type of changes needed would be similar to these

Untested:

diff --git a/pgspecial/dbcommands.py b/pgspecial/dbcommands.py
index 904faac..c52a2ba 100644
--- a/pgspecial/dbcommands.py
+++ b/pgspecial/dbcommands.py
@@ -889,18 +889,20 @@ def describe_table_details(cur, pattern, verbose):
 
 def describe_one_table_details(cur, schema_name, relation_name, oid, verbose):
     if verbose and cur.connection.info.server_version >= 80200:
-        suffix = """pg_catalog.array_to_string(c.reloptions || array(select
-        'toast.' || x from pg_catalog.unnest(tc.reloptions) x), ', ')"""
+        suffix = SQL("""pg_catalog.array_to_string(c.reloptions || array(select
+        'toast.' || x from pg_catalog.unnest(tc.reloptions) x), ', ')""")
     else:
-        suffix = "''"
+        suffix = Literal('')
 
     if cur.connection.info.server_version >= 120000:
-        relhasoids = "false as relhasoids"
+        relhasoids = SQL("false as relhasoids")
     else:
-        relhasoids = "c.relhasoids"
+        relhasoids = SQL("c.relhasoids")
+
+    params = {"suffix": suffix, "relhasoid": relhasoid, "oid": Literal(oid)}
 
     if cur.connection.info.server_version >= 100000:
-        sql = f"""SELECT c.relchecks, c.relkind, c.relhasindex,
+        sql = SQL("""SELECT c.relchecks, c.relkind, c.relhasindex,
                     c.relhasrules, c.relhastriggers, {relhasoids},
                     {suffix},
                     c.reltablespace,
@@ -911,10 +913,10 @@ def describe_one_table_details(cur, schema_name, relation_name, oid, verbose):
                     c.relispartition
                  FROM pg_catalog.pg_class c
                  LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)
-                 WHERE c.oid = '{oid}'"""
+                 WHERE c.oid = {oid}""")
 
     elif cur.connection.info.server_version > 90000:
-        sql = f"""SELECT c.relchecks, c.relkind, c.relhasindex,
+        sql = SQL("""SELECT c.relchecks, c.relkind, c.relhasindex,
                     c.relhasrules, c.relhastriggers, c.relhasoids,
                     {suffix},
                     c.reltablespace,
@@ -925,10 +927,10 @@ def describe_one_table_details(cur, schema_name, relation_name, oid, verbose):
                     false as relispartition
                  FROM pg_catalog.pg_class c
                  LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)
-                 WHERE c.oid = '{oid}'"""
+                 WHERE c.oid = {oid}""")
 
     elif cur.connection.info.server_version >= 80400:
-        sql = f"""SELECT c.relchecks,
+        sql = SQL("""SELECT c.relchecks,
                     c.relkind,
                     c.relhasindex,
                     c.relhasrules,
@@ -941,10 +943,10 @@ def describe_one_table_details(cur, schema_name, relation_name, oid, verbose):
                     false as relispartition
                  FROM pg_catalog.pg_class c
                  LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)
-                 WHERE c.oid = '{oid}'"""
+                 WHERE c.oid = {oid}""")
 
     else:
-        sql = f"""SELECT c.relchecks,
+        sql = SQL("""SELECT c.relchecks,
                     c.relkind,
                     c.relhasindex,
                     c.relhasrules,
@@ -957,7 +959,11 @@ def describe_one_table_details(cur, schema_name, relation_name, oid, verbose):
                     false as relispartition
                  FROM pg_catalog.pg_class c
                  LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)
-                 WHERE c.oid = '{oid}'"""
+                 WHERE c.oid = {oid}""")
+
+    # Calling `as_string()` here  only because the query is passed to
+    # `log.debug()` too. `cur.excute()` can accept the SQL object directly.
+    sql = sql.format(**params).as_string(cur)
 
     # Create a namedtuple called tableinfo and match what's in describe.c
 
@@ -971,8 +977,10 @@ def describe_one_table_details(cur, schema_name, relation_name, oid, verbose):
     # If it's a seq, fetch it's value and store it for later.
     if tableinfo.relkind == "S":
         # Do stuff here.
-        sql = f"""SELECT * FROM "{schema_name}"."{relation_name}"""
-        log.debug(sql)
+        sql = SQL("""SELECT * FROM {schema}.{rel}""").format(
+            schema=Identifier(schema_name), rel=Identifier(relation_name))
+
+        log.debug(sql.as_string(cur))
         cur.execute(sql)
         if not (cur.rowcount > 0):
             return None, None, None, "Something went wrong."

Notice that using the SQL, Identifier, Literal objects etc. there is no more the need to first compose a query as string, with placeholders, and then execute it with params (i.e.. cur.execute(sql % sql_params, params)): SQL.format() can take SQL snippets, identifiers, or literal values and will use the right quotes to compose a query (cur.execute(sql.format(**all_params)))

A problem I see here is with testing: what is test coverage, in terms of supporting all database versions and calling all the commands? If it's low it's dangerous. Maybe it's worth dropping support for old Postgres versions? Psycopg doesn't (officially) support PostgreSQL < 10. Certain things work, but it's untested.

主要言語
Python
スター
86
フォーク
54
PR マージ指標
30日以内にマージされた PR はありません

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

dbcli/pgspecial のほかの issue

dbcli/pgspecial の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。