Add support for 'ONLY' in Index creation dialog. #6386#9643
Open
RohitBhati8269 wants to merge 1 commit intopgadmin-org:masterfrom
Open
Add support for 'ONLY' in Index creation dialog. #6386#9643RohitBhati8269 wants to merge 1 commit intopgadmin-org:masterfrom
RohitBhati8269 wants to merge 1 commit intopgadmin-org:masterfrom
Conversation
WalkthroughAdds an "Only Table?" switch to the Index UI and threads its value into CREATE INDEX templates so the ON clause conditionally includes ONLY when the switch is set. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/tables/indexes/static/js/index.ui.js`:
- Around line 528-535: The indisonly control currently allows being set together
with isconcurrent which produces invalid SQL; update the indisonly field
definition (the disabled callback for indexSchemaObj.node_info) to include deps:
['isconcurrent'] and return true when state.isconcurrent is truthy (preventing
ONLY when CONCURRENTLY is selected), and additionally add a mutual-exclusion
check in the form validate() (or in the index creation validator) to reject the
combination of state.isconcurrent and state.indisonly with a clear validation
error; reference the indisonly field's disabled callback, deps:
['isconcurrent'], state.isconcurrent, and the validate() function to implement
both the UI disable and the validation guard.
- Around line 526-536: The 'indisonly' field in the index field definition is
missing a PostgreSQL version guard; update the field object (id: 'indisonly'
within the index UI schema) to include min_version: 110000 so the switch is only
shown/active for PG >= 11 connections (consistent with other guarded fields like
'deduplicate_items' and 'indnullsnotdistinct'), leaving the existing disabled
and helpMessage logic intact and using indexSchemaObj.node_info for context as
before.
- Line 535: Update the helpMessage for the ONLY option in index.ui.js (the
helpMessage property) to explicitly state that using ONLY creates the index only
on that table and the resulting index is initially marked INVALID; also mention
that the index becomes valid once matching indexes exist on partitions (e.g.,
via ALTER INDEX ... ATTACH PARTITION). Modify the gettext string accordingly so
users know the index won't be usable immediately and how it can be made valid.
...pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js
Show resolved
Hide resolved
...pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js
Show resolved
Hide resolved
RohitBhati8269
force-pushed
the
GH-6386
branch
2 times, most recently
from
d158209 to
ab48397
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tables/indexes/static/js/index.ui.js`:
- Around line 526-537: Update the inline comment above the indisonly switch to
correctly state that ONLY applies to partitioned tables (not inherited tables):
locate the block defining the control with id 'indisonly' (uses
indexSchemaObj.node_info and disabled: () => inSchema(indexSchemaObj.node_info)
|| !indexSchemaObj.node_info?.table?.is_partitioned) and replace the comment
text to: "ONLY is only applicable to partitioned tables".
...pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js
Show resolved
Hide resolved
RohitBhati8269
force-pushed
the
GH-6386
branch
from
ab48397 to
a945776
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/en_US/index_dialog.rst (1)
10-12:⚠️ Potential issue | 🟡 MinorIntro tab list is missing the new With tab.
The introductory sentence still lists only three tabs but a fourth "With" tab has been added (documented at line 69).
📝 Proposed fix
-The *Index* dialog organizes the development of a index through the following -dialog tabs: *General*, *Definition*, and *Columns*. The *SQL* tab displays the SQL code -generated by dialog selections. +The *Index* dialog organizes the development of an index through the following +dialog tabs: *General*, *Definition*, *With*, and *Columns*. The *SQL* tab displays the SQL code +generated by dialog selections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en_US/index_dialog.rst` around lines 10 - 12, The intro sentence for the "Index" dialog currently lists only the three tabs "General", "Definition", and "Columns" (and mentions "SQL") but omits the newly added "With" tab; update that sentence in the "Index dialog" description so the tab list includes "With" (e.g., "General, Definition, Columns, With" and then "SQL" as currently described) to match the later documentation where the "With" tab is documented.
🧹 Nitpick comments (1)
docs/en_US/index_dialog.rst (1)
54-55: Consider noting that "Only Table?" is exclusive to partitioned tables.The UI disables this switch for non-partitioned tables, but the docs don't explain the restriction. Users who see it greyed out will have no guidance.
📝 Suggested wording addition
* Move the switch next to *Only Table?* to the *Yes* position to create the index - only on the parent table without recursing to its partitions. The default is *No*. + only on the parent table without recursing to its partitions. The default is *No*. + This option is only enabled for partitioned tables.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en_US/index_dialog.rst` around lines 54 - 55, The docs mention the "Only Table?" switch but don't say it's only relevant for partitioned tables; update the index_dialog.rst text around the "Only Table?" explanation to add a brief sentence that this option is only applicable to partitioned tables and the UI disables (greys out) the switch for non-partitioned tables so users understand why they may see it inactive; reference the existing phrasing "Only Table?" and the sentence about default being "No" and insert the note immediately after that sentence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/en_US/index_dialog.rst`:
- Around line 10-12: The intro sentence for the "Index" dialog currently lists
only the three tabs "General", "Definition", and "Columns" (and mentions "SQL")
but omits the newly added "With" tab; update that sentence in the "Index dialog"
description so the tab list includes "With" (e.g., "General, Definition,
Columns, With" and then "SQL" as currently described) to match the later
documentation where the "With" tab is documented.
---
Nitpick comments:
In `@docs/en_US/index_dialog.rst`:
- Around line 54-55: The docs mention the "Only Table?" switch but don't say
it's only relevant for partitioned tables; update the index_dialog.rst text
around the "Only Table?" explanation to add a brief sentence that this option is
only applicable to partitioned tables and the UI disables (greys out) the switch
for non-partitioned tables so users understand why they may see it inactive;
reference the existing phrasing "Only Table?" and the sentence about default
being "No" and insert the note immediately after that sentence.
Contributor
Author
|
@anilsahoo20 As per my findings, the predicate functionality for partial indexes already exists in pgAdmin4. |
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.
Summary by CodeRabbit
New Features
Refactor
Documentation