Repository navigation
Preserve opclass options and all storage parameters in reverse engineered index SQL. #9625 #10635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
MaybeSomeone-arc18
wants to merge
2
commits into
pgadmin-org:master
Choose a base branch
from
MaybeSomeone-arc18:fix-9625-index-sql-options
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
175 changes: 175 additions & 0 deletions
175
.../server_groups/servers/databases/schemas/tables/indexes/tests/test_indexes_sql_options.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| ########################################################################## | ||
| # | ||
| # pgAdmin 4 - PostgreSQL Tools | ||
| # | ||
| # Copyright (C) 2013 - 2026, The pgAdmin Development Team | ||
| # This software is released under the PostgreSQL Licence | ||
| # | ||
| ########################################################################## | ||
|
|
||
| """Reverse engineered index SQL must preserve opclass options (e.g. | ||
| gist_trgm_ops(siglen=32)) and every storage parameter found in the | ||
| catalog, including ones pgAdmin does not know about. | ||
|
|
||
| https://github.com/pgadmin-org/pgadmin4/issues/9625 | ||
| """ | ||
|
|
||
| import re | ||
| import uuid | ||
|
|
||
| from pgadmin.browser.server_groups.servers.databases.schemas.tables.columns. \ | ||
| tests import utils as columns_utils | ||
| from pgadmin.browser.server_groups.servers.databases.schemas.tables.tests \ | ||
| import utils as tables_utils | ||
| from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ | ||
| utils as schema_utils | ||
| from pgadmin.browser.server_groups.servers.databases.tests import utils as \ | ||
| database_utils | ||
| from pgadmin.utils.route import BaseTestGenerator | ||
| from regression import parent_node_dict | ||
| from regression.python_test_utils import test_utils as utils | ||
|
|
||
|
|
||
| class IndexSqlOptionsTestCase(BaseTestGenerator): | ||
| """This class tests sql generated for indexes having opclass options | ||
| or storage parameters unknown to pgAdmin.""" | ||
|
|
||
| url = "/browser/index/sql/" | ||
| scenarios = utils.generate_scenarios("index_sql_options", { | ||
| "index_sql_options": [{ | ||
| "name": "Index SQL keeps opclass options and all reloptions", | ||
| "is_positive_test": True, | ||
| "inventory_data": {}, | ||
| "test_data": {}, | ||
| "mocking_required": False, | ||
| "mock_data": {}, | ||
| "expected_data": { | ||
| "status_code": 200, | ||
| "error_msg": None, | ||
| "test_result_data": {} | ||
| }, | ||
| "is_list": False | ||
| }] | ||
| }) | ||
|
|
||
| def setUp(self): | ||
| super().setUp() | ||
| self.db_name = parent_node_dict["database"][-1]["db_name"] | ||
| schema_info = parent_node_dict["schema"][-1] | ||
| self.server_id = schema_info["server_id"] | ||
| self.db_id = schema_info["db_id"] | ||
| db_con = database_utils.connect_database(self, utils.SERVER_GROUP, | ||
| self.server_id, self.db_id) | ||
| if not db_con['data']["connected"]: | ||
| raise Exception("Could not connect to database to add a table.") | ||
| self.schema_id = schema_info["schema_id"] | ||
| self.schema_name = schema_info["schema_name"] | ||
| schema_response = schema_utils.verify_schemas(self.server, | ||
| self.db_name, | ||
| self.schema_name) | ||
| if not schema_response: | ||
| raise Exception("Could not find the schema to add a table.") | ||
| self.table_name = "table_index_options_%s" % (str(uuid.uuid4())[1:8]) | ||
| self.table_id = tables_utils.create_table(self.server, self.db_name, | ||
| self.schema_name, | ||
| self.table_name) | ||
| self.column_name = "content" | ||
| columns_utils.create_column(self.server, self.db_name, | ||
| self.schema_name, self.table_name, | ||
| self.column_name, 'text') | ||
|
|
||
| # Create the indexes with plain SQL: the index dialog supports | ||
| # neither opclass options nor arbitrary storage parameters, which | ||
| # is exactly what this test is about. | ||
| self.opclass_index_name = "idx_opclass_options_%s" % ( | ||
| str(uuid.uuid4())[1:8]) | ||
| self.relopts_index_name = "idx_all_reloptions_%s" % ( | ||
| str(uuid.uuid4())[1:8]) | ||
| connection = utils.get_db_connection(self.db_name, | ||
| self.server['username'], | ||
| self.server['db_password'], | ||
| self.server['host'], | ||
| self.server['port']) | ||
| old_isolation_level = connection.isolation_level | ||
| utils.set_isolation_level(connection, 0) | ||
| pg_cursor = connection.cursor() | ||
| pg_cursor.execute("CREATE EXTENSION IF NOT EXISTS pg_trgm") | ||
| pg_cursor.execute( | ||
| 'ALTER TABLE {0}.{1} ADD COLUMN search_vector tsvector'.format( | ||
| self.schema_name, self.table_name)) | ||
| pg_cursor.execute( | ||
| 'CREATE INDEX {0} ON {1}.{2} USING GIST ' | ||
| '({3} gist_trgm_ops(siglen=32))'.format( | ||
| self.opclass_index_name, self.schema_name, self.table_name, | ||
| self.column_name)) | ||
| pg_cursor.execute( | ||
| 'CREATE INDEX {0} ON {1}.{2} USING btree ({3}) ' | ||
| 'WITH (fillfactor=70, ' | ||
| 'vacuum_cleanup_index_scale_factor=0.5)'.format( | ||
| self.relopts_index_name, self.schema_name, self.table_name, | ||
| self.column_name)) | ||
| self.default_opclass_index_name = "idx_def_opclass_options_%s" % ( | ||
| str(uuid.uuid4())[1:8]) | ||
| pg_cursor.execute( | ||
| 'CREATE INDEX {0} ON {1}.{2} USING GIST ' | ||
| '(search_vector tsvector_ops(siglen=64))'.format( | ||
| self.default_opclass_index_name, self.schema_name, | ||
| self.table_name)) | ||
| pg_cursor.execute( | ||
| "SELECT cls.oid FROM pg_catalog.pg_class cls " | ||
| "JOIN pg_catalog.pg_namespace nsp ON nsp.oid = cls.relnamespace " | ||
| "WHERE cls.relkind = 'i' AND nsp.nspname = %s " | ||
| "AND cls.relname = ANY(%s)", | ||
| (self.schema_name, | ||
| [self.opclass_index_name, self.relopts_index_name, | ||
| self.default_opclass_index_name])) | ||
| self.index_ids = [row[0] for row in pg_cursor.fetchall()] | ||
| utils.set_isolation_level(connection, old_isolation_level) | ||
| connection.close() | ||
| if len(self.index_ids) != 3: | ||
| raise Exception("Could not create the test indexes.") | ||
|
|
||
| def get_index_sql(self, index_id): | ||
| response = self.tester.get( | ||
| "{0}{1}/{2}/{3}/{4}/{5}/{6}".format(self.url, utils.SERVER_GROUP, | ||
| self.server_id, self.db_id, | ||
| self.schema_id, self.table_id, | ||
| index_id), | ||
| follow_redirects=True) | ||
| self.assertEqual(response.status_code, 200) | ||
| resp_sql = response.data.decode('unicode_escape') | ||
| if resp_sql.startswith('"') and resp_sql.endswith('"'): | ||
| resp_sql = resp_sql[1:-1] | ||
| # Collapse whitespace so assertions do not depend on template | ||
| # line breaks. | ||
| return re.sub(r'\s+', ' ', resp_sql) | ||
|
|
||
| def runTest(self): | ||
| if self.is_positive_test: | ||
| sql_by_index = dict( | ||
| (idx, self.get_index_sql(idx)) for idx in self.index_ids) | ||
| opclass_sql = None | ||
| relopts_sql = None | ||
| for idx, sql in sql_by_index.items(): | ||
| if self.opclass_index_name in sql: | ||
| opclass_sql = sql | ||
| if self.relopts_index_name in sql: | ||
| relopts_sql = sql | ||
| self.assertIsNotNone(opclass_sql) | ||
| self.assertIsNotNone(relopts_sql) | ||
| self.assertIn('gist_trgm_ops(siglen=32)', opclass_sql) | ||
| # Options on the default opclass still require the opclass | ||
| # name to be rendered, otherwise the options are lost. | ||
| default_sql = None | ||
| for idx, sql in sql_by_index.items(): | ||
| if self.default_opclass_index_name in sql: | ||
| default_sql = sql | ||
| self.assertIsNotNone(default_sql) | ||
| self.assertIn('tsvector_ops(siglen=64)', default_sql) | ||
| self.assertIn('fillfactor=70', relopts_sql) | ||
| self.assertIn('vacuum_cleanup_index_scale_factor=0.5', | ||
| relopts_sql) | ||
|
|
||
| def tearDown(self): | ||
| # Disconnect the database | ||
| database_utils.disconnect_database(self, self.server_id, self.db_id) |
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: pgadmin-org/pgadmin4
Length of output: 239
🏁 Script executed:
Repository: pgadmin-org/pgadmin4
Length of output: 16890
🌐 Web query:
site:postgresql.org/docs tsvector_ops GiST siglen default operator class💡 Result:
Preserve default opclass names when options are present.
column_details.sqldiscards the opclass name wheno.opcdefaultis true. The create-mode branch then cannot render options for default opclasses such astsvector_ops(siglen=32). Keep the raw opclass name separate so properties mode remains unchanged.🐛 Suggested fix
a.attoptions AS attoptions, + o.opcname AS opclassname, CASE WHEN (o.opcdefault = FALSE) THEN o.opcname ELSE null END AS opcname,🤖 Prompt for AI Agents