Thread: [pgAdmin4][Patch]: Fixed RMs #1220 & #1221
Attachment
Hi Khushboo
I have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.
On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,Khushboo
Akshay Joshi
Sr. Software Architect
Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Mobile: +91 976-788-8246
On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.
Just curious, as I understand the problem, we're not able to able to run pg_dump/pg_restore/psql against the database, which contains '=' in the name.
Can we use PGDATABASE environment variable for them?
Of course - this tools may still fail when special characters (e.g. '=') exists in the name of the database objects (e.g. schema, table, etc).
--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Hi Khushboo,
Could you write tests for this change?
These files are actually apart of the ACITree refactoring patch. If this patch was already in master the task of adding tests would be much easier.
These files are actually apart of the ACITree refactoring patch. If this patch was already in master the task of adding tests would be much easier.
Victoria & Joao
On Wed, May 2, 2018 at 8:33 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.Just curious, as I understand the problem, we're not able to able to run pg_dump/pg_restore/psql against the database, which contains '=' in the name.Can we use PGDATABASE environment variable for them?Of course - this tools may still fail when special characters (e.g. '=') exists in the name of the database objects (e.g. schema, table, etc).--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
On Wed, May 2, 2018 at 6:03 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.Just curious, as I understand the problem, we're not able to able to run pg_dump/pg_restore/psql against the database, which contains '=' in the name.Can we use PGDATABASE environment variable for them?
This way we can implement, but should we consider this way only in case of the database name having "=" ?
Also, the command on the dialogue will not have database name, so user might get confused.
Of course - this tools may still fail when special characters (e.g. '=') exists in the name of the database objects (e.g. schema, table, etc).
It's working with this approach.
--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
On Thu, May 3, 2018 at 12:25 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Wed, May 2, 2018 at 6:03 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.Just curious, as I understand the problem, we're not able to able to run pg_dump/pg_restore/psql against the database, which contains '=' in the name.Can we use PGDATABASE environment variable for them?This way we can implement, but should we consider this way only in case of the database name having "=" ?
I think - we should do it for all for consistent result.
Also, the command on the dialogue will not have database name, so user might get confused.
What need to show as the command line is upto us?
We can also show PGDATABASE=XXXX as environment variable for this command.
Of course - this tools may still fail when special characters (e.g. '=') exists in the name of the database objects (e.g. schema, table, etc).It's working with this approach.
Good to know that.
-- Thanks, Ashesh
--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
On Thu, May 3, 2018 at 7:59 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Thu, May 3, 2018 at 12:25 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 6:03 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.Just curious, as I understand the problem, we're not able to able to run pg_dump/pg_restore/psql against the database, which contains '=' in the name.Can we use PGDATABASE environment variable for them?This way we can implement, but should we consider this way only in case of the database name having "=" ?I think - we should do it for all for consistent result.Also, the command on the dialogue will not have database name, so user might get confused.What need to show as the command line is upto us?We can also show PGDATABASE=XXXX as environment variable for this command.
Cost vs. benefit; how many people actually use = in their database names, and of those, how many will back it up using pgAdmin? My guess is that number is zero, or extremely close to it - and we've only ever had the issue reported by our own QA people who are intentionally trying to break this stuff.
Let's just not support external tools on any objects with = in their names. If there are complaints from users in the future, we can revisit.
Of course - this tools may still fail when special characters (e.g. '=') exists in the name of the database objects (e.g. schema, table, etc).It's working with this approach.Good to know that.-- Thanks, Ashesh--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.
Done. Please find the attached updated patch.
--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Attachment
Hi Victoria,
On Wed, May 2, 2018 at 9:20 PM, Victoria Henry <vhenry@pivotal.io> wrote:
Hi Khushboo,Could you write tests for this change?
We don't have any tests for these utilities, so at this point not possible to write the test cases for the same.
These files are actually apart of the ACITree refactoring patch. If this patch was already in master the task of adding tests would be much easier.Victoria & Joao
Thanks,
Khushboo
On Wed, May 2, 2018 at 8:33 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.Just curious, as I understand the problem, we're not able to able to run pg_dump/pg_restore/psql against the database, which contains '=' in the name.Can we use PGDATABASE environment variable for them?Of course - this tools may still fail when special characters (e.g. '=') exists in the name of the database objects (e.g. schema, table, etc).--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Hi
--
On Thu, May 3, 2018 at 10:19 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.
I changed the messages to read more like this: "Maintenance job creation failed. Databases with = symbols in the name cannot be maintained using this utility.".
However; I think that throwing the error when the user tries to execute the process is quite unhelpful, as the user may have spent some time choosing options etc. Can we do it when they open the dialogue (show the error instead of the tool's dialogue)?
Thanks.
Done. Please find the attached updated patch.--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
Please find the attached updated patch.
On Thu, May 3, 2018 at 9:02 PM, Dave Page <dpage@pgadmin.org> wrote:
HiOn Thu, May 3, 2018 at 10:19 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.I changed the messages to read more like this: "Maintenance job creation failed. Databases with = symbols in the name cannot be maintained using this utility.".
Changed.
However; I think that throwing the error when the user tries to execute the process is quite unhelpful, as the user may have spent some time choosing options etc. Can we do it when they open the dialogue (show the error instead of the tool's dialogue)?
Done.
Thanks.
Thanks,
Khushboo
Done. Please find the attached updated patch.--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment
Thanks patch applied.
On Mon, May 7, 2018 at 12:16 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,Please find the attached updated patch.On Thu, May 3, 2018 at 9:02 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Thu, May 3, 2018 at 10:19 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.I changed the messages to read more like this: "Maintenance job creation failed. Databases with = symbols in the name cannot be maintained using this utility.".Changed.However; I think that throwing the error when the user tries to execute the process is quite unhelpful, as the user may have spent some time choosing options etc. Can we do it when they open the dialogue (show the error instead of the tool's dialogue)?Done.Thanks.Thanks,KhushbooDone. Please find the attached updated patch.--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Akshay Joshi
Sr. Software Architect
Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Mobile: +91 976-788-8246
Could you update the 3.1 release notes to reflect that please?
On Mon, May 7, 2018 at 10:40 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Thanks patch applied.On Mon, May 7, 2018 at 12:16 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached updated patch.On Thu, May 3, 2018 at 9:02 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Thu, May 3, 2018 at 10:19 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.I changed the messages to read more like this: "Maintenance job creation failed. Databases with = symbols in the name cannot be maintained using this utility.".Changed.However; I think that throwing the error when the user tries to execute the process is quite unhelpful, as the user may have spent some time choosing options etc. Can we do it when they open the dialogue (show the error instead of the tool's dialogue)?Done.Thanks.Thanks,KhushbooDone. Please find the attached updated patch.--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, 8 May 2018, 13:35 Dave Page, <dpage@pgadmin.org> wrote:
Could you update the 3.1 release notes to reflect that please?
Sure
On Mon, May 7, 2018 at 10:40 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Thanks patch applied.On Mon, May 7, 2018 at 12:16 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached updated patch.On Thu, May 3, 2018 at 9:02 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Thu, May 3, 2018 at 10:19 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:On Wed, May 2, 2018 at 5:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi KhushbooI have reviewed your code and looks good to me. Can we change the message from "The database name is inappropriate" to some meaningful message, so that user should know why it is inappropriate. If user will be able to create database with "=" in name then why Backup, Maintenance and Restore fails.I changed the messages to read more like this: "Maintenance job creation failed. Databases with = symbols in the name cannot be maintained using this utility.".Changed.However; I think that throwing the error when the user tries to execute the process is quite unhelpful, as the user may have spent some time choosing options etc. Can we do it when they open the dialogue (show the error instead of the tool's dialogue)?Done.Thanks.Thanks,KhushbooDone. Please find the attached updated patch.--On Wed, May 2, 2018 at 3:44 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached patch which will fix RMs # 1220 and #1221.If the database name contains = then the backup, maintenance and restore jobs are failing.To fix these, we will display the error message regarding inappropriate database name.Thanks,KhushbooAkshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company