Skip to content

chore: Add pg_catalog schema prefix to catalog table queries#10004

Open
mzabuawala wants to merge 3 commits into
pgadmin-org:masterfrom
mzabuawala:mz/fix-catalog-table-sql
Open

chore: Add pg_catalog schema prefix to catalog table queries#10004
mzabuawala wants to merge 3 commits into
pgadmin-org:masterfrom
mzabuawala:mz/fix-catalog-table-sql

Conversation

@mzabuawala
Copy link
Copy Markdown
Contributor

@mzabuawala mzabuawala commented Jun 5, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability of database operations by consistently qualifying system catalog references, reducing lookup and compatibility issues across PostgreSQL versions.
  • Refactor

    • Standardized internal SQL/catalog access used for introspection, replication, extensions, and reporting to ensure more consistent, predictable behavior in DB-aware features.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

Walkthrough

This PR consistently schema-qualifies PostgreSQL system catalog references by replacing unqualified pg_* names with pg_catalog.pg_* across SQL templates, runtime Python queries, tests, fixtures, and LLM report queries.

Changes

System Catalog Schema Qualification

Layer / File(s) Summary
Catalog qualification (single checkpoint)
web/... (SQL templates, runtime helpers, tests, fixtures, llm, tools)`
All updated SQL and query strings now reference pg_catalog.* (e.g., pg_extension, pg_attribute, pg_class, pg_type, pg_proc, pg_namespace, pg_stat_activity, pg_stat_replication, pg_replication_slots, pg_available_extensions, etc.) instead of unqualified catalog names while preserving existing query logic and control flow.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • anilsahoo20
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding pg_catalog schema prefixes to catalog table queries throughout the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 82.14% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
web/pgadmin/utils/__init__.py (1)

1001-1011: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

SQL injection risk and line-length violation.

Line 1002 has two issues:

  1. SQL injection vulnerability: The f-string directly interpolates extension_name into the SQL query without parameterization. If extension_name comes from untrusted input, this could allow SQL injection attacks.

  2. Line-length violation: The line exceeds 79 characters (85 > 79), failing pycodestyle (E501).

🔒 Proposed fix using parameterized query
-    sql = f"SELECT * FROM pg_catalog.pg_extension WHERE extname = '{extension_name}'"
+    sql = (
+        "SELECT * FROM pg_catalog.pg_extension "
+        "WHERE extname = %s"
+    )
     status, res = conn.execute_scalar(sql)
+    # Update the execute_scalar call to pass extension_name as a parameter:
+    status, res = conn.execute_scalar(sql, (extension_name,))

Note: You may need to adjust the execute_scalar call depending on how the connection object handles parameters.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/pgadmin/utils/__init__.py` around lines 1001 - 1011, The
check_extension_exists function currently builds SQL via an f-string (SQL
injection risk and too-long line); change it to use a parameterized query and
avoid long lines: construct the SQL as a short string like "SELECT * FROM
pg_catalog.pg_extension WHERE extname = %s" (or the param placeholder your
conn.execute_scalar expects), call conn.execute_scalar(sql, (extension_name,))
instead of interpolating into the string, and reformat the surrounding lines so
no line exceeds 79 chars while preserving the existing status/res return
behavior.
🧹 Nitpick comments (1)
web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/11_plus/get_collation.sql (1)

2-2: ⚡ Quick win

Inconsistent schema casing within the query.

Line 2 uses lowercase pg_catalog while the rest of the query uses uppercase PG_CATALOG (lines 1, 3, 5). Although PostgreSQL treats unquoted identifiers case-insensitively, consistent casing improves readability.

♻️ Proposed fix for consistent casing
-		FROM pg_catalog.PG_PARTITIONED_TABLE PARTT
+		FROM PG_CATALOG.PG_PARTITIONED_TABLE PARTT
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/11_plus/get_collation.sql`
at line 2, Change the inconsistent lowercase schema reference "pg_catalog" to
uppercase "PG_CATALOG" in the SQL fragment (the line containing "FROM
pg_catalog.PG_PARTITIONED_TABLE PARTT") so it matches the other occurrences of
"PG_CATALOG" in the query for consistent casing and readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/tests/utils.py`:
- Around line 125-126: The triple-quoted SQL assignment to variable "query" in
tests/utils.py exceeds the 79-character line length; split the long string into
shorter concatenated pieces (either by using implicit string literal
concatenation inside parentheses or by joining two shorter strings) so the
SELECT/IN clause lines are each ≤79 chars, keeping the variable name "query" and
the same SQL content unchanged.

In
`@web/pgadmin/browser/server_groups/servers/databases/event_triggers/tests/utils.py`:
- Line 118: The string literal containing "(select rolname from
pg_catalog.pg_authid where oid " exceeds the 79-char line limit; locate that
literal in
web/pgadmin/browser/server_groups/servers/databases/event_triggers/tests/utils.py
and break it into shorter pieces using implicit string concatenation or split
the SQL into multiple quoted segments (e.g. "(select rolname " + "from
pg_catalog.pg_authid where oid ") or wrap the full string in parentheses and
place a line break so each line is ≤79 chars; ensure the concatenated value
remains the same and run pycodestyle to confirm the E501 is resolved.

In
`@web/pgadmin/browser/server_groups/servers/databases/languages/tests/utils.py`:
- Line 74: The string literal "(select rolname from pg_catalog.pg_authid where
oid " exceeds the 79-char pycodestyle limit; split this long SQL string into
multiple shorter concatenated or implicitly joined string literals (e.g., break
after "pg_catalog.pg_authid" and continue the rest on the next line inside the
same expression) wherever that literal appears (look for the exact substring
"(select rolname from pg_catalog.pg_authid where oid " in the tests/utils.py
file) so each source line stays <=79 chars.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/tests/utils.py`:
- Around line 73-74: The long SQL invocation in the test helper should be
wrapped to meet the 79-character limit: break the call to pg_cursor.execute(...)
that uses synonym_name into multiple shorter segments (for example split the SQL
string across lines or build the query string on a prior line) so the
pg_cursor.execute call itself stays within 79 chars; keep the same variable
synonym_name and function call pg_cursor.execute to preserve behavior.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/utils.py`:
- Around line 196-197: The SQL call pg_cursor.execute("select * from
pg_catalog.pg_attribute where attname='%s'" % col_name) exceeds the line-length
limit; fix by breaking the query string into multiple concatenated or multi-part
strings passed to pg_cursor.execute (e.g., split the literal before the WHERE
clause or use implicit string concatenation) and keep the format operation with
col_name unchanged so the call still reads pg_cursor.execute(<split-string> %
col_name); update only the string literal to span multiple lines to satisfy E501
for the pg_cursor.execute usage.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py`:
- Around line 35-36: The long SQL string containing "FROM pg_catalog.pg_class
join pg_catalog.pg_attribute on attrelid=pg_class.oid" exceeds 79 chars; split
it into shorter pieces (for example, use implicit concatenation by placing the
parts in parentheses or break into two adjacent string literals like "FROM
pg_catalog.pg_class " "JOIN pg_catalog.pg_attribute ON " "attrelid=pg_class.oid
") so the line length meets pycodestyle (E501) while preserving the exact SQL
text in the code where the JOIN clause string is defined.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_column_acl_sql.py`:
- Around line 31-32: The long SQL string in test_column_acl_sql.py that contains
"FROM pg_catalog.pg_class JOIN pg_catalog.pg_attribute ON attrelid=pg_class.oid"
exceeds the line-length limit; split that SQL literal into smaller string
fragments (use implicit concatenation inside parentheses or separate
+-concatenated literals) so each line stays under the E501 limit, keeping the
SQL tokens intact and preserving whitespace between fragments.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_trigger_get_oid_sql.py`:
- Around line 31-32: The SQL string fragment in test_trigger_get_oid_sql.py
("FROM pg_catalog.pg_class JOIN pg_catalog.pg_attribute ON "
"attrelid=pg_class.oid ") exceeds the line-length limit; fix it by breaking the
long string into shorter concatenated pieces (e.g., split after a logical
boundary like "JOIN" or "ON") or use implicit concatenation inside parentheses
so each source line stays under the E501 limit while preserving the exact SQL
content referenced in the test.

In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py`:
- Around line 613-614: The pg_cursor.execute call with the long SQL string
(pg_cursor.execute("SELECT COUNT(*) FROM pg_catalog.pg_available_extensions
WHERE name ='postgis'")) exceeds the max line length; break the SQL into shorter
literal pieces (e.g. use implicit string joining with adjacent quoted strings or
concatenate two shorter strings) or format it as a multi-line parenthesized
string so the call to pg_cursor.execute remains within line length limits while
keeping the same SQL text.

In `@web/pgadmin/dashboard/__init__.py`:
- Line 701: The SQL assignment to the variable sql currently exceeds the
79-character limit; update the assignment in web/pgadmin/dashboard/__init__.py
so the string is split across multiple lines (use implicit string literal
concatenation in parentheses, a triple-quoted string, or join parts) without
changing the SQL content; locate the sql variable assignment and reformat it
into two or more shorter string pieces (e.g., "SELECT * FROM pg_catalog." +
"pg_extension WHERE extname = 'system_stats';" or use a parenthesized multi-line
string) to satisfy pycodestyle E501.

In `@web/pgadmin/llm/__init__.py`:
- Line 1462: The SQL query string assignments at the schema_query variable and
three other similar query assignments exceed the 79-character line-length limit
set by pycodestyle (E501). Split each of these four long query strings across
multiple lines using implicit string concatenation or continuation to ensure
each line stays within the 79-character limit. Apply this refactoring
consistently at all four locations mentioned (lines 1462, 1541, 1994, and 2073).

In `@web/pgadmin/llm/reports/queries.py`:
- Line 408: The long SQL fragments (e.g. "(SELECT count(*) FROM
pg_catalog.pg_stat_activity) as total_connections," and the other long SELECT
fragment at line 508) exceed the 79-character limit; fix by breaking those long
lines into multiple shorter string pieces or use implicit Python string
concatenation inside parentheses so each physical line is <=79 chars, preserving
the same SQL content and trailing commas/spacing; update the SQL expression(s)
in web/pgadmin/llm/reports/queries.py where these fragments appear to split the
long literal into two or more lines (or join with "+" if needed) without
changing identifiers or alias names.

In `@web/pgadmin/utils/driver/psycopg3/connection.py`:
- Around line 519-521: The inline comment in connection.py (the comment starting
"Note that we use 'UPDATE pg_catalog.pg_settings'...") exceeds the 79-character
line-length rule; split/reflow that comment across multiple shorter lines (<=79
chars) preserving the original wording and meaning, for example breaking after
phrases like "for setting bytea_output as a" and "convenience hack for those
running on old, unsupported versions of PostgreSQL" so each resulting comment
line is within the pycodestyle limit; update the comment near the existing block
in psycopg3 connection.py accordingly.
- Around line 450-451: The SQL assignment to _query in connection.py exceeds the
79-char line limit; update the _query construction in the method where _query is
defined so the SQL literal is split across multiple shorter string segments
(e.g., use implicit string concatenation inside parentheses or split into two
quoted pieces) while still calling self.qtLiteral(role, self.conn) to
interpolate the role value; ensure the resulting _query variable content is
identical but no single source line exceeds 79 characters.

In `@web/regression/python_test_utils/test_utils.py`:
- Around line 1910-1911: The SQL query in check_extension_exists currently
interpolates extension_name into the SQL string (cursor.execute(f"""SELECT
COUNT(*) FROM pg_catalog.pg_extension WHERE extname='{extension_name}'""")),
which risks quoting/SQL issues; change the call to use parameterized execution
instead by passing a query with a placeholder (e.g., WHERE extname = %s) and
supply (extension_name,) as the parameters argument to cursor.execute so the DB
driver handles quoting safely.

In `@web/regression/re_sql/tests/test_resql.py`:
- Around line 851-856: Replace the string-interpolated SQL calls that use
db_name with parameterized queries to avoid SQL injection and long-line issues:
change the two pg_cursor.execute(...) calls that build "SELECT datcollate ...
WHERE datname = '{0}'".format(db_name) and "SELECT datctype ... WHERE datname =
'{0}'".format(db_name) to use bind parameters (e.g. pg_cursor.execute("SELECT
datcollate as cname FROM pg_catalog.pg_database WHERE datname = %s", (db_name,))
and similarly for datctype) and then fetch into lc_collate as before; also break
long lines if needed to satisfy line-length checks.

---

Outside diff comments:
In `@web/pgadmin/utils/__init__.py`:
- Around line 1001-1011: The check_extension_exists function currently builds
SQL via an f-string (SQL injection risk and too-long line); change it to use a
parameterized query and avoid long lines: construct the SQL as a short string
like "SELECT * FROM pg_catalog.pg_extension WHERE extname = %s" (or the param
placeholder your conn.execute_scalar expects), call conn.execute_scalar(sql,
(extension_name,)) instead of interpolating into the string, and reformat the
surrounding lines so no line exceeds 79 chars while preserving the existing
status/res return behavior.

---

Nitpick comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/11_plus/get_collation.sql`:
- Line 2: Change the inconsistent lowercase schema reference "pg_catalog" to
uppercase "PG_CATALOG" in the SQL fragment (the line containing "FROM
pg_catalog.PG_PARTITIONED_TABLE PARTT") so it matches the other occurrences of
"PG_CATALOG" in the query for consistent casing and readability.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 962715c0-a5b1-4a58-87b5-27a6cdbe4d66

📥 Commits

Reviewing files that changed from the base of the PR and between 679c39f and 64cd12d.

📒 Files selected for processing (77)
  • web/pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/__init__.py
  • web/pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/event_triggers/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/templates/foreign_servers/sql/default/delete.sql
  • web/pgadmin/browser/server_groups/servers/databases/languages/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/publications/templates/publications/pg/15_plus/sql/get_tables.sql
  • web/pgadmin/browser/server_groups/servers/databases/publications/templates/publications/pg/default/sql/get_all_columns.sql
  • web/pgadmin/browser/server_groups/servers/databases/publications/templates/publications/pg/default/sql/get_tables.sql
  • web/pgadmin/browser/server_groups/servers/databases/publications/templates/publications/ppas/15_plus/sql/get_tables.sql
  • web/pgadmin/browser/server_groups/servers/databases/publications/templates/publications/ppas/default/sql/get_all_columns.sql
  • web/pgadmin/browser/server_groups/servers/databases/publications/templates/publications/ppas/default/sql/get_tables.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/aggregates/templates/aggregates/sql/11_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/aggregates/templates/aggregates/sql/default/nodes.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/aggregates/templates/aggregates/sql/default/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/foreign_table_columns/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/functions/pg/sql/13_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/functions/pg/sql/14_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/functions/ppas/sql/13_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/functions/ppas/sql/14_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/procedures/pg/sql/13_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/procedures/pg/sql/14_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/procedures/ppas/sql/13_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/procedures/ppas/sql/14_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/indexes/sql/11_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/indexes/sql/13_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/indexes/sql/15_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/indexes/sql/default/nodes.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/indexes/sql/default/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/11_plus/get_collation.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/get_tables_for_constraints.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_tables_for_constraints.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_column_acl_sql.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_trigger_get_oid_sql.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/types/pg/sql/14_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/types/pg/sql/default/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/types/ppas/sql/14_plus/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/types/ppas/sql/default/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/pg/13_plus/sql/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/pg/15_plus/sql/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/ppas/13_plus/sql/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/ppas/15_plus/sql/properties.sql
  • web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/view_test_data.json
  • web/pgadmin/browser/server_groups/servers/databases/subscriptions/templates/subscriptions/sql/default/stats.sql
  • web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/15_plus/get_icu_locale.sql
  • web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/16_plus/get_ctypes.sql
  • web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/17_plus/get_builtin_locale.sql
  • web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/17_plus/get_ctypes.sql
  • web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/17_plus/get_icu_locale.sql
  • web/pgadmin/browser/server_groups/servers/directories/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/replica_nodes/templates/replica_nodes/sql/default/count.sql
  • web/pgadmin/browser/server_groups/servers/replica_nodes/templates/replica_nodes/sql/default/nodes.sql
  • web/pgadmin/browser/server_groups/servers/replica_nodes/templates/replica_nodes/sql/default/properties.sql
  • web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/default/properties.sql
  • web/pgadmin/browser/server_groups/servers/templates/servers/sql/default/replication_type.sql
  • web/pgadmin/browser/server_groups/servers/tests/servers_test_data.json
  • web/pgadmin/dashboard/__init__.py
  • web/pgadmin/dashboard/templates/dashboard/sql/default/replication_slots.sql
  • web/pgadmin/dashboard/templates/dashboard/sql/default/replication_stats.sql
  • web/pgadmin/llm/__init__.py
  • web/pgadmin/llm/reports/generator.py
  • web/pgadmin/llm/reports/queries.py
  • web/pgadmin/tools/schema_diff/tests/pg/12_plus/source.sql
  • web/pgadmin/tools/schema_diff/tests/pg/17_plus/source.sql
  • web/pgadmin/tools/search_objects/templates/search_objects/sql/pg/11_plus/search.sql
  • web/pgadmin/tools/search_objects/templates/search_objects/sql/pg/default/search.sql
  • web/pgadmin/tools/search_objects/templates/search_objects/sql/ppas/11_plus/search.sql
  • web/pgadmin/tools/search_objects/templates/search_objects/sql/ppas/12_plus/search.sql
  • web/pgadmin/tools/search_objects/templates/search_objects/sql/ppas/default/search.sql
  • web/pgadmin/utils/__init__.py
  • web/pgadmin/utils/driver/psycopg3/connection.py
  • web/regression/python_test_utils/test_utils.py
  • web/regression/re_sql/tests/test_resql.py

Comment thread web/pgadmin/browser/server_groups/servers/databases/event_triggers/tests/utils.py Outdated
Comment thread web/pgadmin/browser/server_groups/servers/databases/languages/tests/utils.py Outdated
Comment thread web/pgadmin/llm/reports/queries.py Outdated
Comment thread web/pgadmin/utils/driver/psycopg3/connection.py Outdated
Comment thread web/pgadmin/utils/driver/psycopg3/connection.py Outdated
Comment thread web/regression/python_test_utils/test_utils.py
Comment thread web/regression/re_sql/tests/test_resql.py Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
web/pgadmin/utils/driver/psycopg3/connection.py (2)

450-452: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix line-length violation.

Line 450 exceeds the 79-character limit (84 characters). Break the SQL string across multiple lines to comply with pycodestyle (E501).

🔧 Proposed fix to split the SQL string
-            _query = "SELECT rolname from pg_catalog.pg_roles " \
-                     "WHERE rolname = {0}" \
-                     "".format(self.qtLiteral(role, self.conn))
+            _query = (
+                "SELECT rolname FROM pg_catalog.pg_roles "
+                "WHERE rolname = {0}"
+            ).format(self.qtLiteral(role, self.conn))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/pgadmin/utils/driver/psycopg3/connection.py` around lines 450 - 452, The
SQL assignment to _query in connection.py exceeds the 79-char limit; split the
SQL string into multiple shorter parts (using implicit string literal
concatenation or enclosing the full string in parentheses) so no line is longer
than 79 chars, preserving the call to self.qtLiteral(role, self.conn) and the
overall format: "_query = ... .format(self.qtLiteral(role, self.conn))"; update
only the string literal splitting around "SELECT rolname from
pg_catalog.pg_roles" and the WHERE clause so pycodestyle E501 is satisfied while
keeping the same SQL content and .format call.

520-523: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix line-length violation in comment.

Line 519 (preceding the visible change) exceeds the 79-character limit (88 characters). Reflow the comment across multiple lines to comply with pycodestyle (E501).

🔧 Proposed fix to reflow the comment
-        # Note that we use 'UPDATE pg_catalog.pg_settings' for setting bytea_output as a
-        # convenience hack for those running on old, unsupported versions of
-        # PostgreSQL 'cos we're nice like that.
+        # Note that we use 'UPDATE pg_catalog.pg_settings' for setting
+        # bytea_output as a convenience hack for those running on old,
+        # unsupported versions of PostgreSQL 'cos we're nice like that.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/pgadmin/utils/driver/psycopg3/connection.py` around lines 520 - 523, The
long comment explaining the use of "UPDATE pg_catalog.pg_settings" to set
bytea_output exceeds 79 chars; reflow it into multiple shorter lines (each ≤79
chars) while preserving the original wording and meaning, e.g. break the
sentence after logical phrases so the lines stay within the pycodestyle limit;
update the comment block that mentions "UPDATE pg_catalog.pg_settings" and
"bytea_output" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/tests/utils.py`:
- Around line 73-75: verify_synonym and create_synonym currently build SQL with
Python % string interpolation which risks SQL injection; change the value-based
WHERE clauses (e.g., "WHERE synname='%s'") to use parameterized execution (pass
synonym_name as a query parameter to pg_cursor.execute instead of formatting
into the string), and for SQL that embeds identifiers (CREATE OR REPLACE SYNONYM
%s.%s FOR %s.%s and the SELECT s.oid ... WHERE s.synname='%s' in create_synonym)
switch to psycopg.sql composition using psycopg.sql.Identifier and
psycopg.sql.SQL to safely quote schema/table/synonym identifiers (construct the
SQL with SQL() and Identifier() and pass parameters where appropriate) so no
user values are interpolated directly into the query string.

In `@web/pgadmin/utils/__init__.py`:
- Around line 1002-1005: The SQL currently interpolates extension_name directly
into the string (variable sql) causing SQL injection; change to a parameterized
query and pass extension_name via the conn.execute_scalar params argument
instead of f-string interpolation (e.g. use "SELECT * FROM
pg_catalog.pg_extension WHERE extname = %s" and call conn.execute_scalar with
the SQL and the extension_name parameter). Ensure you update the code paths that
call conn.execute_scalar so they pass the parameter rather than using the
f-string.

---

Duplicate comments:
In `@web/pgadmin/utils/driver/psycopg3/connection.py`:
- Around line 450-452: The SQL assignment to _query in connection.py exceeds the
79-char limit; split the SQL string into multiple shorter parts (using implicit
string literal concatenation or enclosing the full string in parentheses) so no
line is longer than 79 chars, preserving the call to self.qtLiteral(role,
self.conn) and the overall format: "_query = ... .format(self.qtLiteral(role,
self.conn))"; update only the string literal splitting around "SELECT rolname
from pg_catalog.pg_roles" and the WHERE clause so pycodestyle E501 is satisfied
while keeping the same SQL content and .format call.
- Around line 520-523: The long comment explaining the use of "UPDATE
pg_catalog.pg_settings" to set bytea_output exceeds 79 chars; reflow it into
multiple shorter lines (each ≤79 chars) while preserving the original wording
and meaning, e.g. break the sentence after logical phrases so the lines stay
within the pycodestyle limit; update the comment block that mentions "UPDATE
pg_catalog.pg_settings" and "bytea_output" accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fdaa05c4-a4a3-4599-becc-40fe246e973b

📥 Commits

Reviewing files that changed from the base of the PR and between 6f9bb55 and cd1892d.

📒 Files selected for processing (16)
  • web/pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/event_triggers/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/languages/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/foreign_table_columns/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_column_acl_sql.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_trigger_get_oid_sql.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py
  • web/pgadmin/dashboard/__init__.py
  • web/pgadmin/llm/__init__.py
  • web/pgadmin/llm/reports/queries.py
  • web/pgadmin/utils/__init__.py
  • web/pgadmin/utils/driver/psycopg3/connection.py
  • web/regression/re_sql/tests/test_resql.py
✅ Files skipped from review due to trivial changes (2)
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_trigger_get_oid_sql.py
🚧 Files skipped from review as they are similar to previous changes (8)
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_column_acl_sql.py
  • web/pgadmin/browser/server_groups/servers/databases/languages/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/tests/utils.py
  • web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py
  • web/pgadmin/dashboard/init.py
  • web/pgadmin/browser/server_groups/servers/databases/event_triggers/tests/utils.py
  • web/pgadmin/llm/reports/queries.py
  • web/pgadmin/llm/init.py

Comment thread web/pgadmin/utils/__init__.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant