Skip to content

Conversation

@khushboovashi
Copy link
Contributor

@khushboovashi khushboovashi commented Feb 12, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Corrected handling of numeric values so query results and CSV exports display numeric columns without quotes and with proper formatting.
  • Tests

    • Updated CSV download tests to cover numeric column handling and made assertions more robust to avoid brittle failures.

@coderabbitai
Copy link

coderabbitai bot commented Feb 12, 2026

Walkthrough

Adds a cursor-level numeric typecaster registration function and invokes it in the psycopg3 connection flow; updates a SQL editor CSV download test to include a numeric "Price" column and adjust assertions accordingly.

Changes

Cohort / File(s) Summary
Numeric typecasters
web/pgadmin/utils/driver/psycopg3/typecast.py
Adds register_numeric_typecasters(_cursor) registering NumericLoader, FloatLoader, and IntLoader for OIDs 1700, 700/701, and 20 on a cursor adapter.
Connection usage
web/pgadmin/utils/driver/psycopg3/connection.py
Imports and calls register_numeric_typecasters(cur) after cursor scroll in gen() so numeric columns are fetched without quoting.
Test update
web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py
Expands test query to include 2300::numeric AS "Price", updates expected CSV columns/values, and replaces strict boolean containment assertions with assertIn membership checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 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 fix: resolving an issue where the 'Quote strings only' configuration was being ignored during result set downloads, which aligns with the changes that add numeric typecaster registration to properly handle numeric values without unwanted quoting.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py (1)

174-175: Consider adding a brief docstring to register_numeric_typecasters.

The existing comment on line 173 is slightly ambiguous. A one-line docstring like """Register original numeric loaders at the cursor level to bypass global text-casting.""" would make the intent clearer for future maintainers, consistent with how register_string_typecasters and register_binary_typecasters are documented via their comments.

Also applies to: 186-187


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

@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

🤖 Fix all issues with AI agents
In `@web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py`:
- Line 32: The SQL string passed as the sql= parameter in
test_download_csv_query_tool.py exceeds 79 chars; break it into multiple shorter
string literals (e.g., wrap the argument in parentheses and use implicit
concatenation or split into two/three quoted pieces) so each line is under 79
characters while preserving the exact SQL content (including quotes around
column aliases and the numeric cast "2300::numeric as \"Price\""). Update the
sql= argument in the test function to use the multi-line string form so
pycodestyle E501 is resolved.

In `@web/pgadmin/utils/driver/psycopg3/typecast.py`:
- Around line 172-183: The continuation lines in register_numeric_typecasters
are over-indented causing pycodestyle E127; reformat the
_cursor.adapters.register_loader calls so continuation lines align under the
opening parenthesis instead of being visually over-indented—e.g., for each call
in register_numeric_typecasters (the calls that register NumericLoader,
FloatLoader, IntLoader) move the second-line indentation to match the position
after the opening parenthesis of _cursor.adapters.register_loader or place the
arguments on the same line to satisfy E127.
🧹 Nitpick comments (2)
web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py (1)

173-174: Consider also updating remaining assertTrue(... in ...) calls to assertIn.

Lines 173-174 and 185-186 in the other test branches still use self.assertTrue(self.output_columns in csv_data). For consistency with the changes on lines 208-209, these could be updated too.

Also applies to: 185-186

web/pgadmin/utils/driver/psycopg3/typecast.py (1)

172-183: Consider adding OID 23 (int4) and OID 21 (int2) for consistency with bigint registration.

OID 23 and OID 21 are not currently registered in register_numeric_typecasters, nor are they listed in TO_STRING_NUMERIC_DATATYPES. They will use psycopg3's default loaders, which return native Python integers—so they work correctly out of the box. However, for consistency with OID 20 (bigint), which explicitly uses IntLoader at the cursor level to override the global TextLoaderpgAdmin registration, you may want to add similar registrations for int4 and int2.

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