Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Generate sql commands using `psycopg.sql`

Aperta
#134 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
30/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Ferma
Stack tecnologico
postgresql, python
Ambito
cli, databases

Direzione di ricerca

Inizia ispezionando pgspecial/dbcommands.py, in particolare describe_one_table_details, e individua gli altri punti di composizione delle stringhe SQL nelle implementazioni dei comandi. Esamina come sono coperte le diramazioni in base alla versione di PostgreSQL e l’esecuzione dei comandi. Il lavoro è completato quando i comandi generati rilevanti usano psycopg.sql in modo sicuro, preservando il comportamento supportato dei comandi e una copertura adeguata tra le versioni del database.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.

Lingua principale
Python
Stelle
86
Fork
54
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di dbcli/pgspecial

Tutte le issue di dbcli/pgspecial

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.