Feat: Added a Statistics node #2018#9748
Open
mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
Open
Feat: Added a Statistics node #2018#9748mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
Conversation
WalkthroughAdds a new "Statistics" schema child node with backend REST endpoints, SQL templates for multiple PostgreSQL versions, frontend UI components, tests and test utilities, and integrates the module into the schema module and webpack build. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (UI)
participant Server as pgAdmin REST
participant DB as PostgreSQL
Browser->>Server: POST /schema/<scid>/statistics (create payload)
Server->>DB: Execute CREATE STATISTICS (version-specific SQL)
DB-->>Server: Success + optionally new OID
Server-->>Browser: 201 Created (node info or success)
Browser->>Server: GET /schema/<scid>/statistics (list)
Server->>DB: Query pg_statistic_ext (nodes/properties templates)
DB-->>Server: Rows (statistics metadata)
Server-->>Browser: 200 OK (list payload)
Browser->>Server: PUT /schema/<scid>/statistics/<stid> (update)
Server->>DB: Execute ALTER/COMMENT SQL (templated)
DB-->>Server: Success
Server-->>Browser: 200 OK (updated node)
Browser->>Server: DELETE /schema/<scid>/statistics/<stid>
Server->>DB: Execute DROP STATISTICS (cascade/only handling)
DB-->>Server: Success
Server-->>Browser: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
mzabuawala
force-pushed
the
Feat_STATISTICS
branch
8 times, most recently
from
0c50973 to
ab59af0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (9)
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/update.sql (1)
3-7: Redundant conditional produces identical output.The
{% if %}and{% else %}branches on line 4 emit the same SQL:{{ conn|qtIdent(o_data.schema, o_data.name) }}. The conditional is dead code and can be simplified.♻️ Proposed simplification
{### Rename statistics ###} {% if data.name and data.name != o_data.name %} -ALTER STATISTICS {% if data.schema and data.schema != o_data.schema %}{{ conn|qtIdent(o_data.schema, o_data.name) }}{% else %}{{ conn|qtIdent(o_data.schema, o_data.name) }}{% endif %} +ALTER STATISTICS {{ conn|qtIdent(o_data.schema, o_data.name) }} RENAME TO {{ conn|qtIdent(data.name) }};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/update.sql` around lines 3 - 7, The conditional around emitting the identifier in the ALTER STATISTICS statement is redundant because both branches output {{ conn|qtIdent(o_data.schema, o_data.name) }}; remove the entire {% if data.schema and data.schema != o_data.schema %} ... {% else %} ... {% endif %} block and replace it with a single occurrence of {{ conn|qtIdent(o_data.schema, o_data.name) }} so ALTER STATISTICS always uses that identifier (refer to the ALTER STATISTICS line and the conn|qtIdent(o_data.schema, o_data.name) expression).web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/backend_support.sql (1)
1-5: LGTM with a minor optimization suggestion.The logic correctly detects PostgreSQL 10+ extended statistics support. Using
EXISTSwould be slightly more efficient as it short-circuits on the first match:♻️ Optional: Use EXISTS for efficiency
{### Check if extended statistics are supported (PostgreSQL 10+) ###} SELECT - CASE WHEN COUNT(*) > 0 THEN TRUE ELSE FALSE END AS has_statistics -FROM pg_catalog.pg_class -WHERE relname='pg_statistic_ext' + EXISTS ( + SELECT 1 FROM pg_catalog.pg_class WHERE relname = 'pg_statistic_ext' + ) AS has_statistics🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/backend_support.sql` around lines 1 - 5, Replace the COUNT-based detection of extended statistics with an EXISTS-based check to short-circuit on the first match: update the query that produces has_statistics (currently scanning pg_catalog.pg_class with WHERE relname='pg_statistic_ext') to use an EXISTS subquery so it returns TRUE as soon as a matching pg_statistic_ext row is found, keeping the same column name has_statistics and still querying pg_catalog.pg_class with relname='pg_statistic_ext'.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sql (1)
3-7: Redundant conditional produces identical output.Same issue as in the 17_plus template: both branches output
{{ conn|qtIdent(o_data.schema, o_data.name) }}.♻️ Proposed simplification
{### Rename statistics ###} {% if data.name and data.name != o_data.name %} -ALTER STATISTICS {% if data.schema and data.schema != o_data.schema %}{{ conn|qtIdent(o_data.schema, o_data.name) }}{% else %}{{ conn|qtIdent(o_data.schema, o_data.name) }}{% endif %} +ALTER STATISTICS {{ conn|qtIdent(o_data.schema, o_data.name) }} RENAME TO {{ conn|qtIdent(data.name) }};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sql` around lines 3 - 7, The inner conditional in the update.sql template is redundant because both branches render {{ conn|qtIdent(o_data.schema, o_data.name) }}; simplify by removing the conditional and directly use {{ conn|qtIdent(o_data.schema, o_data.name) }} where the ALTER STATISTICS target is built (keep the outer check {% if data.name and data.name != o_data.name %} ... RENAME TO {{ conn|qtIdent(data.name) }}; intact); no other logic changes required.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sql (1)
1-1: Misleading version comment for a default template.The comment states "PostgreSQL 10-11" but this file is in the
defaultfolder, which typically serves as the fallback for versions without specific templates. Consider either moving this to a version-specific folder (e.g.,10_plus) or updating the comment to reflect its actual usage scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sql` at line 1, The comment in the stats.sql default template incorrectly labels the template as "PostgreSQL 10-11"; either remove or update that version-specific wording in the stats.sql file (the default template for statistics) to indicate it is a generic/fallback template, or move this template into a version-specific directory (e.g., the 10_plus template set) so the comment accurately reflects its scope; update the header comment in stats.sql (or relocate the file) and ensure the comment matches the template's actual usage.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.json (1)
232-249: Limited test coverage for update operations.The
statistics_putsuite only tests renaming. Consider adding test cases for:
- Schema change
- Owner change
- Comment update
- Non-existing object (negative test, similar to delete/get suites)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.json` around lines 232 - 249, Add broader update test cases to the statistics_put suite: keep the existing "Update statistics: Change name." case and add tests that exercise schema change (set a new "schema" in test_data), owner change (set an "owner" value), comment update (set a "comment" field), and a negative test where the object does not exist (use the same "url": "/browser/statistics/obj/" but set an invalid identifier in inventory_data or test_data and assert a non-200 status and error_msg). Ensure each new case follows the same shape as the existing entry (fields: name, url, is_positive_test, inventory_data, test_data, mocking_required, mock_data, expected_data) and uses unique descriptive names so they run alongside the current "Update statistics: Change name." case.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/create.sql (1)
1-14: Remove duplicate template; 17_plus/create.sql differs from 16_plus/create.sql only in the version comment.The two templates have identical SQL logic. If PostgreSQL 17 introduced no syntax changes for
CREATE STATISTICS, remove the 17_plus template and let template resolution fall back to 16_plus.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/create.sql` around lines 1 - 14, The 17_plus/create.sql template duplicates the CREATE STATISTICS logic from 16_plus/create.sql (the CREATE STATISTICS block and COMMENT ON STATISTICS handling), so delete the redundant 17_plus/create.sql file and rely on template resolution to fall back to 16_plus/create.sql; ensure no unique content from 17_plus (e.g., version-only comment) is required elsewhere and update any references or tests that explicitly point to the 17_plus template to use the fallback or 16_plus instead.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/properties.sql (1)
1-1: Minor: Inaccurate comment for the version-specific template.The comment states "PostgreSQL 14+" but this template is in the
16_plusfolder. Consider updating the comment to reflect this is for PostgreSQL 16+.-{### Query extended statistics properties from pg_statistic_ext (PostgreSQL 14+) ###} +{### Query extended statistics properties from pg_statistic_ext (PostgreSQL 16+) ###}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/properties.sql` at line 1, The header comment in the template is inaccurate — update the comment string "{### Query extended statistics properties from pg_statistic_ext (PostgreSQL 14+) ###}" to reference PostgreSQL 16+ (e.g. change "PostgreSQL 14+" to "PostgreSQL 16+") so it correctly reflects the file's 16_plus template; ensure the modified comment remains the same format and location.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py (1)
75-75: Unused variabledb_user.The variable
db_useris assigned but never used in the method.def runTest(self): """This function will add statistics under schema node.""" - db_user = self.server["username"] self.data["schema"] = self.schema_name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py` at line 75, The assignment to db_user in the test (db_user = self.server["username"]) is unused; remove this unused variable assignment from the test in tests/test_statistics_add.py, or if the intent was to reference the server username later, replace usages to use db_user consistently (e.g., use db_user where self.server["username"] is currently repeated) so there are no unused variables.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/properties.sql (1)
1-1: Minor: Update version comment to match template location.The comment indicates "PostgreSQL 14+" but this template resides in the
17_plusfolder.-{### Query extended statistics properties from pg_statistic_ext (PostgreSQL 14+) ###} +{### Query extended statistics properties from pg_statistic_ext (PostgreSQL 17+) ###}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/properties.sql` at line 1, Update the version comment at the top of the template: replace "PostgreSQL 14+" with "PostgreSQL 17+" so the header in the statistics SQL template in the 17_plus folder accurately reflects the template location; ensure the comment string in the file that currently reads "{### Query extended statistics properties from pg_statistic_ext (PostgreSQL 14+) ###}" is edited to say "(PostgreSQL 17+)" to keep versioning consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py`:
- Around line 422-435: The current parsing uses split(',') which breaks SQL
expressions with commas; update the parsing in the block that checks
has_expression_list to stop naive comma-splitting: if data['expression_list'] is
provided, first try to json.loads it as an array of expressions and map each
element to {'expression': <elem>}; if json.loads fails, treat the entire
expression_list string as a single expression (wrap it as [{'expression':
expression_list}]) instead of splitting; keep the existing assignment to
data['expressions'] but remove the list comprehension that splits on ',' and add
validation/logging for malformed JSON so callers (UI) are encouraged to send a
structured list.
- Around line 667-677: The function msql() currently assigns the second value
from self.get_SQL(...) to the name "_" which shadows the module-level gettext
alias and causes an UnboundLocalError when _("...") is called during validation;
change the assignment "sql, _ = self.get_SQL(gid, sid, did, data, scid, stid)"
to use a different local name (e.g., "sql, sql_params" or "sql, dummy") and
update any subsequent uses of that second value accordingly so "_" remains the
gettext function and is not shadowed.
- Around line 565-570: The render_template calls that build the DELETE SQL
(using self.template_path and self._DELETE_SQL) do not pass the DB connection
into the template context, but the delete.sql uses {{ conn|qtIdent(schema, name)
}}; update both render_template invocations (the one around the block
referencing res['rows'][0]['name']/['schema'] and the other similar call later)
to include conn=self.conn in their keyword args so the qtIdent filter can access
the connection.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.ui.js`:
- Around line 80-127: Add a dependency-driven reset for schema changes: in the
field definition with id 'schema' add a depChange handler that clears 'table'
and 'columns' (e.g., return {table: null, columns: []}) so stale selections
cannot persist when schema changes; also update the 'table' field's options
loader usage (the options or optionsLoaded logic that currently uses
this.fieldOptions.tables / obj.allTablesOptions) so it takes the selected schema
from state (use the same state.schema value or call
obj.getTableOid/state.schema) when fetching tables, and update the table-loader
in statistics.js to request tables scoped to the selected schema rather than a
global list. Ensure references: id 'schema', id 'table', id 'columns',
obj.getTableOid, and obj.fieldOptions.getColumns are used to locate and change
the code.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sql`:
- Line 1: Update the misleading header comment "{### SQL to update extended
statistics object (PostgreSQL 13+) ###}" to reflect the directory name by
changing "PostgreSQL 13+" to "PostgreSQL 16+" so the comment matches the folder
`16_plus`; locate the comment in the template file's top-level SQL header (the
exact string above) and replace the version number accordingly.
- Around line 22-25: Validate and cast data.stattarget to an integer before
rendering this template: ensure wherever request.form/request.json populates
data.stattarget you apply explicit int() casting and/or numeric validation and
reject non-integer input, so the template variable data.stattarget used in the
ALTER STATISTICS ... SET STATISTICS statement is guaranteed to be an integer;
keep the existing use of qtIdent(...) for schema/name but enforce integer
semantics for data.stattarget and return a validation error if casting fails.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sql`:
- Around line 14-18: The stxkind CASE in the stats.sql template is missing the
mapping for PostgreSQL 15's 'e' kind (expression statistics); update the CASE
that checks "kind" (the stxkind case in the statistics/sql/default/stats.sql
template) to include WHEN 'e' THEN 'expressions' so expression statistics are
returned by the default template fallback used for versions that don't override
this file.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.json`:
- Around line 74-77: Update the inaccurate skip message for the inventory_data
test fixture: locate the JSON object named "inventory_data" (which currently
sets "server_min_version": 140000) and change its "skip_msg" to either reflect
that MCV extended statistics were introduced in PostgreSQL 12 (e.g., "MCV
statistics introduced in PG 12") or remove the message entirely if the
server_min_version is intended to indicate a different PG14+ requirement; ensure
the skip_msg matches the actual gating condition tied to server_min_version.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py`:
- Around line 103-106: The test uses eval(self.mock_data["return_value"]) inside
the patch side_effect which is unsafe; replace eval with a safe parser or
factory – e.g., use ast.literal_eval or json.loads on
self.mock_data["return_value"] (or create a centralized factory function that
maps known mock keys to concrete return objects) and pass that parsed value as
the side_effect in the patch for self.mock_data["function_name"], leaving
statistics_utils.api_create(self) call unchanged; ensure tests that set
mock_data return_value produce JSON/text or keys compatible with the chosen
parser/factory and update any test fixtures accordingly.
- Around line 31-33: The setUp method in StatisticsAddTestCase is missing a call
to its parent initializer; update StatisticsAddTestCase.setUp to call
super().setUp() at the start (so BaseTestGenerator and other test setup logic
runs) before assigning self.data = self.test_data, ensuring proper test
initialization.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete_multiple.py`:
- Around line 117-120: The test is using an invalid MIME type 'html/json' for
the JSON POST payload; locate the call that passes content_type='html/json' (in
the test in test_statistics_delete_multiple.py where the client request is made
with follow_redirects=True, data=data, content_type=...) and change the
content_type value to 'application/json' so the request uses the correct JSON
Content-Type header.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete.py`:
- Around line 129-134: The test currently only verifies deletion of
self.statistics_name but misses verifying self.statistics_name_2 when
self.is_list is true; update the teardown/backend verification in
test_statistics_delete (after calling statistics_utils.verify_statistics) to
also call statistics_utils.verify_statistics(self.server, self.db_name,
self.statistics_name_2) and assert it is None when self.is_list is True (i.e.,
only perform the second check when setUp() created self.statistics_name_2);
ensure both assertions use self.assertIsNone with descriptive messages so a
partial batch-delete (first removed, second present) will fail the test.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_get.py`:
- Around line 134-138: The test uses eval(self.mock_data["return_value"]) which
is unsafe; replace it with a safe parser such as
ast.literal_eval(self.mock_data["return_value"]) after importing ast, or better
yet return a concrete object via a small factory/mapping keyed by
self.mock_data["return_value"] and referenced in the patch call; update the
patch call in test_statistics_get.py (the block using
self.mock_data["function_name"] and statistics_utils.api_get(self)) to use the
safe parsed/constructed object instead of eval and ensure the corresponding
import or factory mapping is added to the test file.
- Around line 58-64: The code reads server_con["data"]["version"] before
verifying the connection succeeded, which can raise a KeyError if connect_server
failed; update the logic in the block that calls
server_utils.connect_server(self, self.server_id) so you first check
server_con.get("info") == "Server connected." (or existence of "data") and only
then read ver = server_con["data"]["version"]; if the check fails, raise the
exception immediately, otherwise compare ver to self.data["server_max_version"]
and call self.skipTest(self.data["skip_msg"]) as before.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_put.py`:
- Around line 112-116: The test uses eval(self.mock_data["return_value"]) inside
the patch side_effect (in test_statistics_put.py) which is a security risk;
replace eval with a safe factory/parser: add a helper like
create_side_effect(config) (or reuse ast.literal_eval if values are simple
literals) that inspects self.mock_data (keys "type"/"value"/"class"/"message" or
"value") and returns either an exception instance or a plain value, then change
the patch to use side_effect=[create_side_effect(self.mock_data)] referencing
the same function name used in the diff instead of eval; ensure the helper
handles exception creation (using getattr on builtins for class names) and
simple value returns to avoid executing arbitrary code.
- Around line 110-120: The test currently only sets response when
self.mocking_required is True, so if self.is_positive_test is False and
self.mocking_required is False response remains undefined; ensure response is
always assigned by calling statistics_utils.api_put(self) in the missing branch
(i.e., add an else branch to the if self.mocking_required block or initialize
response before the conditional) so response is defined before
utils.assert_status_code and utils.assert_error_message; reference: response,
self.mocking_required, self.is_positive_test, and statistics_utils.api_put.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.py`:
- Around line 153-189: The create_statistics() helper opens a DB connection via
test_utils.get_db_connection and currently only commits and returns the
statistics OID on the happy path, leaking connections on errors; update
create_statistics() to ensure connection cleanup by moving
test_utils.set_isolation_level(connection, old_isolation_level),
connection.commit() and connection.close() into a finally block (or closing the
cursor/connection in finally) so the connection is always closed even on
exceptions, and apply the same finally-based cleanup pattern to other helpers
that use test_utils.get_db_connection, pg_cursor, connection, and
test_utils.set_isolation_level.
---
Nitpick comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/properties.sql`:
- Line 1: The header comment in the template is inaccurate — update the comment
string "{### Query extended statistics properties from pg_statistic_ext
(PostgreSQL 14+) ###}" to reference PostgreSQL 16+ (e.g. change "PostgreSQL 14+"
to "PostgreSQL 16+") so it correctly reflects the file's 16_plus template;
ensure the modified comment remains the same format and location.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sql`:
- Around line 3-7: The inner conditional in the update.sql template is redundant
because both branches render {{ conn|qtIdent(o_data.schema, o_data.name) }};
simplify by removing the conditional and directly use {{
conn|qtIdent(o_data.schema, o_data.name) }} where the ALTER STATISTICS target is
built (keep the outer check {% if data.name and data.name != o_data.name %} ...
RENAME TO {{ conn|qtIdent(data.name) }}; intact); no other logic changes
required.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/create.sql`:
- Around line 1-14: The 17_plus/create.sql template duplicates the CREATE
STATISTICS logic from 16_plus/create.sql (the CREATE STATISTICS block and
COMMENT ON STATISTICS handling), so delete the redundant 17_plus/create.sql file
and rely on template resolution to fall back to 16_plus/create.sql; ensure no
unique content from 17_plus (e.g., version-only comment) is required elsewhere
and update any references or tests that explicitly point to the 17_plus template
to use the fallback or 16_plus instead.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/properties.sql`:
- Line 1: Update the version comment at the top of the template: replace
"PostgreSQL 14+" with "PostgreSQL 17+" so the header in the statistics SQL
template in the 17_plus folder accurately reflects the template location; ensure
the comment string in the file that currently reads "{### Query extended
statistics properties from pg_statistic_ext (PostgreSQL 14+) ###}" is edited to
say "(PostgreSQL 17+)" to keep versioning consistent.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/update.sql`:
- Around line 3-7: The conditional around emitting the identifier in the ALTER
STATISTICS statement is redundant because both branches output {{
conn|qtIdent(o_data.schema, o_data.name) }}; remove the entire {% if data.schema
and data.schema != o_data.schema %} ... {% else %} ... {% endif %} block and
replace it with a single occurrence of {{ conn|qtIdent(o_data.schema,
o_data.name) }} so ALTER STATISTICS always uses that identifier (refer to the
ALTER STATISTICS line and the conn|qtIdent(o_data.schema, o_data.name)
expression).
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/backend_support.sql`:
- Around line 1-5: Replace the COUNT-based detection of extended statistics with
an EXISTS-based check to short-circuit on the first match: update the query that
produces has_statistics (currently scanning pg_catalog.pg_class with WHERE
relname='pg_statistic_ext') to use an EXISTS subquery so it returns TRUE as soon
as a matching pg_statistic_ext row is found, keeping the same column name
has_statistics and still querying pg_catalog.pg_class with
relname='pg_statistic_ext'.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sql`:
- Line 1: The comment in the stats.sql default template incorrectly labels the
template as "PostgreSQL 10-11"; either remove or update that version-specific
wording in the stats.sql file (the default template for statistics) to indicate
it is a generic/fallback template, or move this template into a version-specific
directory (e.g., the 10_plus template set) so the comment accurately reflects
its scope; update the header comment in stats.sql (or relocate the file) and
ensure the comment matches the template's actual usage.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.json`:
- Around line 232-249: Add broader update test cases to the statistics_put
suite: keep the existing "Update statistics: Change name." case and add tests
that exercise schema change (set a new "schema" in test_data), owner change (set
an "owner" value), comment update (set a "comment" field), and a negative test
where the object does not exist (use the same "url": "/browser/statistics/obj/"
but set an invalid identifier in inventory_data or test_data and assert a
non-200 status and error_msg). Ensure each new case follows the same shape as
the existing entry (fields: name, url, is_positive_test, inventory_data,
test_data, mocking_required, mock_data, expected_data) and uses unique
descriptive names so they run alongside the current "Update statistics: Change
name." case.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py`:
- Line 75: The assignment to db_user in the test (db_user =
self.server["username"]) is unused; remove this unused variable assignment from
the test in tests/test_statistics_add.py, or if the intent was to reference the
server username later, replace usages to use db_user consistently (e.g., use
db_user where self.server["username"] is currently repeated) so there are no
unused variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9674af98-86d0-4fcb-8398-4e95fbb92be2
⛔ Files ignored due to path filters (2)
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/img/coll-statistics.svgis excluded by!**/*.svgweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/img/statistics.svgis excluded by!**/*.svg
📒 Files selected for processing (32)
web/pgadmin/browser/server_groups/servers/databases/schemas/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.jsweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.ui.jsweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/properties.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/properties.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/update.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/backend_support.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/coll_stats.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/count.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/delete.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/get_oid.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/nodes.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/properties.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/update.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/pg/14_plus/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/pg/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.jsonweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete_multiple.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_get.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_put.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.pyweb/webpack.config.jsweb/webpack.shim.js
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py
Show resolved
Hide resolved
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py
Show resolved
Hide resolved
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py
Outdated
Show resolved
Hide resolved
...gadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.ui.js
Show resolved
Hide resolved
...rver_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sql
Outdated
Show resolved
Hide resolved
...dmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_get.py
Show resolved
Hide resolved
...dmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_get.py
Show resolved
Hide resolved
...dmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_put.py
Show resolved
Hide resolved
...dmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_put.py
Show resolved
Hide resolved
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.py
Show resolved
Hide resolved
mzabuawala
force-pushed
the
Feat_STATISTICS
branch
2 times, most recently
from
a757d95 to
fe79b94
Compare
mzabuawala
force-pushed
the
Feat_STATISTICS
branch
from
fe79b94 to
25bb575
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py (1)
31-33:⚠️ Potential issue | 🟠 MajorCall
super().setUp()before usingBaseTestGeneratorstate.This is the only new statistics test that skips the base initializer, so later accesses to scenario/base attributes can run against partially initialized state.
Minimal fix
def setUp(self): + super().setUp() # Load test data self.data = self.test_data🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py` around lines 31 - 33, The setUp method in the test (function setUp) uses BaseTestGenerator state before initializing it; modify setUp to call super().setUp() as the first action so the base class initializer runs before accessing self.test_data or other scenario/base attributes (i.e., add a call to super().setUp() at the top of setUp in the TestStatisticsAdd test file).web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py (2)
890-895:⚠️ Potential issue | 🟠 MajorPass
conninto the schema-diff DROP render.The
target_onlydiff path still rendersdelete.sqlwithoutconn=self.conn, soqtIdentcannot quote the qualified statistics name there.Minimal fix
sql = render_template( "/".join([self.template_path, self._DELETE_SQL]), name=target_params['name'], schema=target_params['schema'], - cascade=False + cascade=False, + conn=self.conn )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py` around lines 890 - 895, The target-only diff branch renders the delete.sql template without the DB connection, so qtIdent cannot properly quote the qualified statistics name; update the render_template call that builds sql (using self.template_path and self._DELETE_SQL) to pass conn=self.conn (i.e., render_template(..., name=..., schema=..., cascade=False, conn=self.conn)) so the template can use the connection for quoting.
422-435:⚠️ Potential issue | 🟠 MajorStop parsing expressions with
split(','), and reuse the same normalizer in preview SQL.
coalesce(a, b)becomes two broken entries here, andmsql()renders the same CREATE template without even buildingdata['expressions']. Extract one normalization step and use it for both create and preview paths instead of comma-splitting free text.Also applies to: 644-679, 728-733
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py` around lines 422 - 435, Replace the ad-hoc comma split logic that builds data['expressions'] from data['expression_list'] with a shared normalization helper so complex expressions like "coalesce(a, b)" are preserved; create a function (e.g. normalize_expressions) that takes the raw expression_list string, parses/normalizes it into a list of {'expression': ...} entries (preserving parentheses and function calls, trimming whitespace, and ignoring empty items) and call this helper wherever the preview/build path currently handles expression_list (the code that assigns data['expressions'] from data['expression_list'] and the preview path that uses msql), removing any split(',') usage and ensuring both creation and preview use the same normalized output.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.py (1)
93-133:⚠️ Potential issue | 🟠 MajorClose these DB helpers in
finally.These helpers only close the connection on the success path. Any exception before
connection.close()leaks a backend session, and the helpers that switch isolation level can also skip restoring it. Move cleanup intofinallyforcreate_table_for_statistics(),create_statistics(),verify_statistics(),get_statistics_id(),delete_statistics(), andget_statistics_columns().Also applies to: 153-190, 205-224, 239-259, 275-296, 311-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.py` around lines 93 - 133, The helpers (create_table_for_statistics, create_statistics, verify_statistics, get_statistics_id, delete_statistics, get_statistics_columns) currently only close connections on the success path; move the cleanup into a finally block: ensure any created cursor is closed, the connection isolation level is restored to old_isolation_level if set, and the connection is closed in finally even if exceptions occur; also ensure you only call set_isolation_level and commit/rollback when connection is not None and handle exceptions during commit by rolling back before closing to avoid leaking backend sessions.web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete.py (1)
129-134:⚠️ Potential issue | 🟠 MajorVerify the second statistics object in list-delete mode.
When
self.is_listis true, the positive path only checksself.statistics_name. A batch delete that leavesself.statistics_name_2behind will still pass.Minimal fix
statistics_response = statistics_utils.verify_statistics( self.server, self.db_name, self.statistics_name ) self.assertIsNone(statistics_response, "Deleted statistics still present") + if self.is_list: + statistics_response = statistics_utils.verify_statistics( + self.server, self.db_name, self.statistics_name_2 + ) + self.assertIsNone( + statistics_response, + "Second deleted statistics still present" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete.py` around lines 129 - 134, The test only verifies deletion of self.statistics_name when self.is_list is True, allowing self.statistics_name_2 to remain; update the teardown/assertion to also verify that self.statistics_name_2 is deleted in list-delete mode by calling statistics_utils.verify_statistics for self.statistics_name_2 (in addition to self.statistics_name) and asserting the response is None; modify the block that currently calls statistics_utils.verify_statistics(self.server, self.db_name, self.statistics_name) to perform the same verification for self.statistics_name_2 when self.is_list is True.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete_multiple.py`:
- Around line 111-121: The test currently only asserts a 200 response after
calling self.tester.delete with data=json.dumps({'ids': self.statistics_ids})
but doesn't verify backend deletion; after the delete request (the call to
self.tester.delete in test_statistics_delete_multiple.py), re-query the two
statistics by their generated names/IDs (referencing self.statistics_ids and any
helper used to fetch statistics in the test class) and assert that neither
exists (e.g., returns None or 404) to ensure both records were removed rather
than relying solely on response.status_code.
---
Duplicate comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py`:
- Around line 890-895: The target-only diff branch renders the delete.sql
template without the DB connection, so qtIdent cannot properly quote the
qualified statistics name; update the render_template call that builds sql
(using self.template_path and self._DELETE_SQL) to pass conn=self.conn (i.e.,
render_template(..., name=..., schema=..., cascade=False, conn=self.conn)) so
the template can use the connection for quoting.
- Around line 422-435: Replace the ad-hoc comma split logic that builds
data['expressions'] from data['expression_list'] with a shared normalization
helper so complex expressions like "coalesce(a, b)" are preserved; create a
function (e.g. normalize_expressions) that takes the raw expression_list string,
parses/normalizes it into a list of {'expression': ...} entries (preserving
parentheses and function calls, trimming whitespace, and ignoring empty items)
and call this helper wherever the preview/build path currently handles
expression_list (the code that assigns data['expressions'] from
data['expression_list'] and the preview path that uses msql), removing any
split(',') usage and ensuring both creation and preview use the same normalized
output.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.py`:
- Around line 31-33: The setUp method in the test (function setUp) uses
BaseTestGenerator state before initializing it; modify setUp to call
super().setUp() as the first action so the base class initializer runs before
accessing self.test_data or other scenario/base attributes (i.e., add a call to
super().setUp() at the top of setUp in the TestStatisticsAdd test file).
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete.py`:
- Around line 129-134: The test only verifies deletion of self.statistics_name
when self.is_list is True, allowing self.statistics_name_2 to remain; update the
teardown/assertion to also verify that self.statistics_name_2 is deleted in
list-delete mode by calling statistics_utils.verify_statistics for
self.statistics_name_2 (in addition to self.statistics_name) and asserting the
response is None; modify the block that currently calls
statistics_utils.verify_statistics(self.server, self.db_name,
self.statistics_name) to perform the same verification for
self.statistics_name_2 when self.is_list is True.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.py`:
- Around line 93-133: The helpers (create_table_for_statistics,
create_statistics, verify_statistics, get_statistics_id, delete_statistics,
get_statistics_columns) currently only close connections on the success path;
move the cleanup into a finally block: ensure any created cursor is closed, the
connection isolation level is restored to old_isolation_level if set, and the
connection is closed in finally even if exceptions occur; also ensure you only
call set_isolation_level and commit/rollback when connection is not None and
handle exceptions during commit by rolling back before closing to avoid leaking
backend sessions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 559c1b33-8f6b-404a-82fc-f369275fd60b
⛔ Files ignored due to path filters (2)
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/img/coll-statistics.svgis excluded by!**/*.svgweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/img/statistics.svgis excluded by!**/*.svg
📒 Files selected for processing (32)
web/pgadmin/browser/server_groups/servers/databases/schemas/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.jsweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.ui.jsweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/properties.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/properties.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/update.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/backend_support.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/coll_stats.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/count.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/delete.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/get_oid.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/nodes.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/properties.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/update.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/pg/14_plus/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/pg/__init__.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.jsonweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_add.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete_multiple.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_get.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/test_statistics_put.pyweb/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/utils.pyweb/webpack.config.jsweb/webpack.shim.js
✅ Files skipped from review due to trivial changes (14)
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/count.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/coll_stats.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/backend_support.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/get_oid.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/nodes.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/stats.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/create.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/create.sql
- web/webpack.config.js
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/create.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/properties.sql
- web/webpack.shim.js
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/16_plus/update.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/update.sql
🚧 Files skipped from review as they are similar to previous changes (7)
- web/pgadmin/browser/server_groups/servers/databases/schemas/init.py
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/delete.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/update.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/default/properties.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/templates/statistics/sql/17_plus/properties.sql
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/static/js/statistics.ui.js
- web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/tests/statistics_test_data.json
web/pgadmin/browser/server_groups/servers/databases/schemas/statistics/__init__.py
Show resolved
Hide resolved
.../server_groups/servers/databases/schemas/statistics/tests/test_statistics_delete_multiple.py
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes - #2018
Summary by CodeRabbit
New Features
Tests