Re: [PROPOSAL] new diagnostic items for the dynamic sql - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: [PROPOSAL] new diagnostic items for the dynamic sql
Date
Msg-id CAFj8pRB78tFdXuPDeCE3_JP21ksdursM15zEXphXghgzT2=H9g@mail.gmail.com
Whole thread Raw
In response to Re: [PROPOSAL] new diagnostic items for the dynamic sql  (Dinesh Chemuduru <dinesh.kumar@migops.com>)
Responses Re: [PROPOSAL] new diagnostic items for the dynamic sql
List pgsql-hackers
Hi

I tested the last patch, and I think I found unwanted behavior.

The value of PG_SQL_TEXT is not empty only when the error is related to the parser stage. When the error is raised in the query evaluation stage, then the value is empty.
I think this is too confusing. PL/pgSQL is a high level language, and the behaviour should be consistent independent of internal implementation. I am afraid this feature requires much more work.

postgres=# DO $$
DECLARE
  err_sql_stmt TEXT;
  err_sql_pos INT;
BEGIN
  EXECUTE 'SELECT 1/0';
EXCEPTION
  WHEN OTHERS THEN
    GET STACKED DIAGNOSTICS
      err_sql_stmt = PG_SQL_TEXT,
      err_sql_pos = PG_ERROR_LOCATION;
    RAISE NOTICE 'exception sql "%"', err_sql_stmt;
    RAISE NOTICE 'exception sql position %', err_sql_pos;
END;
$$;
NOTICE:  exception sql ""
NOTICE:  exception sql position 0
DO

For this case, the empty result is not acceptable in this language. It is too confusing. The implemented behaviour is well described in regress tests, but I don't think it is user (developer) friendly. The location field is not important, and can be 0 some times. But query text should be not empty in all possible cases related to any query evaluation. I think this can be a nice and useful feature, but the behavior should be consistent.

Second, but minor, objection to this patch is zero formatting in a regress test.

Regards

Pavel

pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: Improve logging when using Huge Pages
Next
From: Kyotaro Horiguchi
Date:
Subject: Show redo LSN in checkpoint logs