Обсуждение: [pgAdmin4][Patch]: Fix the logic to extract the error in Query tool

Hi Dave,

PFA patch.

On Mon, Sep 18, 2017 at 4:34 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Sep 18, 2017 at 10:54 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi,

PFA minor patch to fix the issue where logic to extract the error using RegEX from error message was incorrect in Query tool(History tab).

RM#2700

Thanks - applied, but....

- Could you please add some JS tests to ensure parseErrorMessage continues to work as it should.

​Done​

 

- I'm not happy with the fact that we still display:

can't execute an empty query ********** Error **********

Can we not make that look more like a real error message? At the very least something like:

Error: can't execute an empty query

​Done​

 

 

Another minor issue which I observed on login page is that close button on alert is little misaligned(screenshot attached).

Also applied - thanks! 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

On Mon, Sep 18, 2017 at 2:20 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Please disregard my previous patch and instead attaching updated patch.

In my previous patch I used `let` keyword instead of `var` for defining variable, for consistency & backward compatibility I have used `var` in my latest patch.

That seems like an odd way to fix this - we put in ***** Error ***** in the backend, then remove it in the front end.

I think it would be better to ensure it's formatted sensibly at the backed to begin with. 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi Dave,

Please find updated patch, I have tested following scenarios in query tool,

1) To test if we highlight faulty syntax  

SQL: select a from pg_roles;

2) To check duplicates in error messages.

- Open query tool

- Uncheck Auto-Commit and run below sql 3 times and you will get an error

SQL: select a from pg_roles;

3) To check proper error

SQL: --insert into pg_roles values(1);

4) To check duplicates in error messages.

SQL: insert into pg_roles values(1);

5) Tested RAISE notices from function.

6) Tested JS testcases

Please review and let me know if I missed anything.

Regards,

Murtuza

On Mon, Sep 18, 2017 at 8:20 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Sep 18, 2017 at 3:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Sorry my bad, I didn't check the backend code, I assumed that it is coming from psycopg2 and so I was focusing it to remove from client side :(

PFA updated patch.

I think it needs to be a bit smarter than that. Whilst it works well for the "empty query" message, it doesn't work well for errors that have full details. For instance, instead of:

========================================================

ERROR:  relation "pg_foo" does not exist

LINE 1: select * from pg_foo

                      ^

********** Error **********

ERROR: relation "pg_foo" does not exist

SQL state: 42P01

Character: 15

========================================================

We now get:

========================================================

ERROR: ERROR:  relation "pg_foo" does not exist

LINE 1: select * from pg_foo

                      ^

ERROR: relation "pg_foo" does not exist

SQL state: 42P01

Character: 15

======================================================== 

​Done​

 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company