Skip to content
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

directory_table: matchignore & remove meta-command username output #464

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

edespino
Copy link
Contributor

@edespino edespino commented Jun 8, 2024

Why are the changes needed

  • If a developer runs the directory_table test suite using a non-gpadmin account, a diff will be encountered. The \dY meta-command output and NOTICE|DETAIL|ERROR output contained gpadmin hardcoded values.

Corrective Action

  • As they introduce the username of the active user account running the tests, remove '\dY' (List of relations) meta-commands. Subsequent SQL commands exist which validate similar output (e.g. display row counts) .

  • Update the pg_regress init_file to include matchignore entries to ignore the usernames in storage user mapping NOTICE, DETAIL & ERROR messages.

How was this patch tested?

  • Ran installcheck as both a dev user (eespino) and as gpadmin. Both times, the directory_table test passed.

Example of hardcoded gpadmin values encountered when tests are run as a non-gpadmin user (eespino)

diff -I HINT: -I CONTEXT: -I GP_IGNORE: -U3 /opt/src/cloudberrydb/src/test/regress/expected/directory_table_optimizer.out /opt/src/cloudberrydb/src/test/regress/results/directory_table.out
--- /opt/src/cloudberrydb/src/test/regress/expected/directory_table_optimizer.out       2024-06-07 22:51:25.569817129 +0000
+++ /opt/src/cloudberrydb/src/test/regress/results/directory_table.out  2024-06-07 22:51:25.620817129 +0000
@@ -270,17 +270,17 @@
 CREATE STORAGE USER MAPPING FOR CURRENT_USER STORAGE SERVER oss_server1;
 CREATE STORAGE USER MAPPING FOR CURRENT_USER STORAGE SERVER oss_server1
 OPTIONS (accesskey 'KGFQWEFQQEFXVAEAWLLC', secretkey '0SJIWiIATh6jOlmAKr8DGq6hOAGBI1BnsnvgJmTs');   -- fail
-ERROR:  storage user mapping for "gpadmin" already exists for storage server "oss_server1"
+ERROR:  storage user mapping for "eespino" already exists for storage server "oss_server1"
 CREATE STORAGE USER MAPPING IF NOT EXISTS FOR CURRENT_USER STORAGE SERVER oss_server1;
-NOTICE:  storage user mapping for "gpadmin" already exists for storage server "oss_server1", skipping
+NOTICE:  storage user mapping for "eespino" already exists for storage server "oss_server1", skipping
 CREATE STORAGE USER MAPPING IF NOT EXISTS FOR CURRENT_USER STORAGE SERVER oss_server1
 OPTIONS (auth_method 'simple');
-NOTICE:  storage user mapping for "gpadmin" already exists for storage server "oss_server1", skipping
+NOTICE:  storage user mapping for "eespino" already exists for storage server "oss_server1", skipping
 CREATE STORAGE USER MAPPING FOR CURRENT_ROLE STORAGE SERVER oss_server2
 OPTIONS (accesskey 'EQEJIOJFAKQWESQEJIWQ', secretkey '0ADQiAxcaUJ2lMHipis80hsUEhdiqui82JhduOKE');
 CREATE STORAGE USER MAPPING FOR CURRENT_USER STORAGE SERVER oss_server2
 OPTIONS (accesskey 'KGFQWEFQQEFXVAEAWLLC', secretkey '0SJIWiIATh6jOlmAKr8DGq6hOAGBI1BnsnvgJmTs');   -- fail
-ERROR:  storage user mapping for "gpadmin" already exists for storage server "oss_server2"
+ERROR:  storage user mapping for "eespino" already exists for storage server "oss_server2"
 CREATE STORAGE USER MAPPING FOR CURRENT_ROLE STORAGE SERVER oss_server3
 OPTIONS (accesskey 'EQEJIOJFAKQWESQEJIWQ', secretkey '0ADQiAxcaUJ2lMHipis80hsUEhdiqui82JhduOKE');
 CREATE STORAGE USER MAPPING FOR CURRENT_USER STORAGE SERVER oss_not_exits;  -- fail
@@ -303,7 +303,7 @@
 ERROR:  role "no_exist_user" does not exist
 CREATE STORAGE USER MAPPING IF NOT EXISTS FOR CURRENT_USER STORAGE SERVER oss_server1
 OPTIONS (accesskey 'KGFQWEFQQEFXVAEAWLLC', secretkey '0SJIWiIATh6jOlmAKr8DGq6hOAGBI1BnsnvgJmTs');   -- skip
-NOTICE:  storage user mapping for "gpadmin" already exists for storage server "oss_server1", skipping
+NOTICE:  storage user mapping for "eespino" already exists for storage server "oss_server1", skipping
 CREATE STORAGE USER MAPPING IF NOT EXISTS FOR test_dirtable2 STORAGE SERVER oss_server3
 OPTIONS (endpoint '127.0.0.1:6555');
 CREATE STORAGE USER MAPPING IF NOT EXISTS FOR test_dirtable3 STORAGE SERVER oss_server8
@@ -436,13 +436,13 @@
 \c regression
 -- Test DROP STOARGE SERVER
 DROP STORAGE SERVER oss_server1;    -- fail
-DETAIL:  storage server of storage user mapping for gpadmin on storage server oss_server1
+DETAIL:  storage server of storage user mapping for eespino on storage server oss_server1
 ERROR:  storage server "oss_server1" cannot be dropped because some objects depend on it
 DROP STORAGE SERVER oss_server2;    -- fail
 DETAIL:  storage server of storage user mapping for test_dirtable1 on storage server oss_server2
 ERROR:  storage server "oss_server2" cannot be dropped because some objects depend on it
 DROP STORAGE SERVER oss_server3;    -- fail
-DETAIL:  storage server of storage user mapping for gpadmin on storage server oss_server3
+DETAIL:  storage server of storage user mapping for eespino on storage server oss_server3
 ERROR:  storage server "oss_server3" cannot be dropped because some objects depend on it
 storage server of storage user mapping for test_dirtable1 on storage server oss_server3
 storage server of storage user mapping for test_dirtable2 on storage server oss_server3
@@ -528,12 +528,12 @@
                 List of relations
  Schema |    Name    |      Type       |  Owner
 --------+------------+-----------------+---------
- public | dir_table1 | directory table | gpadmin
- public | dir_table2 | directory table | gpadmin
- public | dir_table3 | directory table | gpadmin
- public | dir_table4 | directory table | gpadmin
- public | dir_table5 | directory table | gpadmin
- public | dir_table6 | directory table | gpadmin
+ public | dir_table1 | directory table | eespino
+ public | dir_table2 | directory table | eespino
+ public | dir_table3 | directory table | eespino
+ public | dir_table4 | directory table | eespino
+ public | dir_table5 | directory table | eespino
+ public | dir_table6 | directory table | eespino
 (6 rows)

 SELECT count(*) FROM pg_directory_table;
@@ -638,10 +638,10 @@
                 List of relations
  Schema |    Name    |      Type       |  Owner
 --------+------------+-----------------+---------
- public | dir_table1 | directory table | gpadmin
- public | dir_table2 | directory table | gpadmin
- public | dir_table3 | directory table | gpadmin
- public | dir_table6 | directory table | gpadmin
+ public | dir_table1 | directory table | eespino
+ public | dir_table2 | directory table | eespino
+ public | dir_table3 | directory table | eespino
+ public | dir_table6 | directory table | eespino
 (4 rows)

 SELECT count(*) FROM pg_directory_table;
@@ -1771,7 +1771,7 @@
 DROP FUNCTION IF EXISTS trigtest;
 DROP STORAGE USER MAPPING IF EXISTS FOR CURRENT_USER STORAGE SERVER oss_server1;
 DROP STORAGE USER MAPPING IF EXISTS FOR CURRENT_USER STORAGE SERVER oss_server2;
-NOTICE:  storage user mapping for "gpadmin" does not exist for storage server "oss_server2", skipping
+NOTICE:  storage user mapping for "eespino" does not exist for storage server "oss_server2", skipping
 DROP STORAGE USER MAPPING IF EXISTS FOR CURRENT_USER STORAGE SERVER oss_server3;
 DROP STORAGE USER MAPPING IF EXISTS FOR CURRENT_USER STORAGE SERVER oss_server4;
 NOTICE:  storage server "oss_server4" does not exist, skipping

PROBLEM

* If a developer runs the `directory_table` test suite using a
  non-gpadmin account, a diff will be encountered. The `\dY`
  meta-command output and NOTICE|DETAIL|ERROR output contained
  `gpadmin` hardcoded values.

CORRECTIVE ACTION

* As they introduce the username of the active user account running
  the tests, remove '\dY' (List of relations) meta-commands.
  Subsequent SQL commands exist which validate similar
  output (e.g. display row counts) .

* Update the pg_regress init_file to include matchignore entries to
  ignore the usernames in storage user mapping NOTICE, DETAIL & ERROR
  messages.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hiiii, @edespino welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!

@wenchaozhang-123
Copy link
Contributor

LGTM.

@my-ship-it
Copy link
Contributor

Good contribution, thanks, Ed!

@my-ship-it my-ship-it merged commit 6678582 into apache:main Jun 11, 2024
9 of 10 checks passed
@tuhaihe
Copy link
Member

tuhaihe commented Jun 11, 2024

@edespino Thanks for your contribution!

@edespino edespino deleted the directory_table_test_fix branch June 21, 2024 19:28
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.

6 participants