New error code to track unsupported contexts

Started by Michael Paquierabout 11 years ago6 messages
#1Michael Paquier
michael.paquier@gmail.com
1 attachment(s)

Hi all,

When pg_event_trigger_dropped_objects is run in a context that is not
the one of an event trigger, currently the error code
ERRCODE_FEATURE_NOT_SUPPORTED is returned. Wouldn't it be better to
have an error to define an out-of-context instead? It seems that it
would be a good thing to have more error verbosity for situations like
the case above. Note that this idea has been mentioned on this ML a
couple of weeks back. In any case, attached is a patch showing the
idea.

Opinions? Is that worth having?
Regards,
--
Michael

Attachments:

20141129_context_error_code.patchtext/x-diff; charset=US-ASCII; name=20141129_context_error_code.patchDownload
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 6a3002f..154bed9 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1218,7 +1218,7 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
 	if (!currentEventTriggerState ||
 		!currentEventTriggerState->in_sql_drop)
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				(errcode(ERRCODE_CONTEXT_NOT_SUPPORTED),
 		 errmsg("%s can only be called in a sql_drop event trigger function",
 				"pg_event_trigger_dropped_objects()")));
 
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index 62ba092..dcd8489 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -114,6 +114,7 @@ Section: Class 09 - Triggered Action Exception
 Section: Class 0A - Feature Not Supported
 
 0A000    E    ERRCODE_FEATURE_NOT_SUPPORTED                                  feature_not_supported
+0A001    E    ERRCODE_CONTEXT_NOT_SUPPORTED                                  context_not_supported
 
 Section: Class 0B - Invalid Transaction Initiation
 
#2Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Michael Paquier (#1)
Re: New error code to track unsupported contexts

On 11/28/14 11:41 PM, Michael Paquier wrote:

Hi all,

When pg_event_trigger_dropped_objects is run in a context that is not
the one of an event trigger, currently the error code
ERRCODE_FEATURE_NOT_SUPPORTED is returned. Wouldn't it be better to
have an error to define an out-of-context instead? It seems that it
would be a good thing to have more error verbosity for situations like
the case above. Note that this idea has been mentioned on this ML a
couple of weeks back. In any case, attached is a patch showing the
idea.

Opinions? Is that worth having?

Anything ever happen with this? (FWIW, I'm in favor of it. Reporting the
feature isn't supported is confusing...)
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Jim Nasby (#2)
Re: New error code to track unsupported contexts

Jim Nasby wrote:

On 11/28/14 11:41 PM, Michael Paquier wrote:

Hi all,

When pg_event_trigger_dropped_objects is run in a context that is not
the one of an event trigger, currently the error code
ERRCODE_FEATURE_NOT_SUPPORTED is returned. Wouldn't it be better to
have an error to define an out-of-context instead? It seems that it
would be a good thing to have more error verbosity for situations like
the case above. Note that this idea has been mentioned on this ML a
couple of weeks back. In any case, attached is a patch showing the
idea.

Opinions? Is that worth having?

Anything ever happen with this? (FWIW, I'm in favor of it. Reporting the
feature isn't supported is confusing...)

Not opposed to the idea.

Maybe it should be in class 39 'External Routine Invocation Exception'
instead, like ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED is used by
various trigger functions. We could invent
ERRCODE_E_R_I_E_EVENT_TRIGGER_PROTOCOL_VIOLATED with value 39P03, for
example.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Alvaro Herrera (#3)
1 attachment(s)
Re: New error code to track unsupported contexts

On Wed, Apr 8, 2015 at 10:21 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Jim Nasby wrote:

On 11/28/14 11:41 PM, Michael Paquier wrote:

Hi all,

When pg_event_trigger_dropped_objects is run in a context that is not
the one of an event trigger, currently the error code
ERRCODE_FEATURE_NOT_SUPPORTED is returned. Wouldn't it be better to
have an error to define an out-of-context instead? It seems that it
would be a good thing to have more error verbosity for situations like
the case above. Note that this idea has been mentioned on this ML a
couple of weeks back. In any case, attached is a patch showing the
idea.

Opinions? Is that worth having?

Anything ever happen with this? (FWIW, I'm in favor of it. Reporting the
feature isn't supported is confusing...)

This got lost in translation..

Not opposed to the idea.

Maybe it should be in class 39 'External Routine Invocation Exception'
instead, like ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED is used by
various trigger functions. We could invent
ERRCODE_E_R_I_E_EVENT_TRIGGER_PROTOCOL_VIOLATED with value 39P03, for
example.

This looks fine to me, in the spirit of
ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED. A refreshed patch gives the
attached, taking into account the event table_rewrite.
Regards,
--
Michael

Attachments:

20150407_context_error_code_v2.patchapplication/x-patch; name=20150407_context_error_code_v2.patchDownload
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 0026e53..f07fd06 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1416,7 +1416,7 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
 	if (!currentEventTriggerState ||
 		!currentEventTriggerState->in_sql_drop)
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				(errcode(ERRCODE_E_R_I_E_EVENT_TRIGGER_PROTOCOL_VIOLATED),
 		 errmsg("%s can only be called in a sql_drop event trigger function",
 				"pg_event_trigger_dropped_objects()")));
 
@@ -1536,7 +1536,7 @@ pg_event_trigger_table_rewrite_oid(PG_FUNCTION_ARGS)
 	if (!currentEventTriggerState ||
 		currentEventTriggerState->table_rewrite_oid == InvalidOid)
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				(errcode(ERRCODE_E_R_I_E_EVENT_TRIGGER_PROTOCOL_VIOLATED),
 		 errmsg("%s can only be called in a table_rewrite event trigger function",
 				"pg_event_trigger_table_rewrite_oid()")));
 
@@ -1557,7 +1557,7 @@ pg_event_trigger_table_rewrite_reason(PG_FUNCTION_ARGS)
 	if (!currentEventTriggerState ||
 		currentEventTriggerState->table_rewrite_reason == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				(errcode(ERRCODE_E_R_I_E_EVENT_TRIGGER_PROTOCOL_VIOLATED),
 		 errmsg("%s can only be called in a table_rewrite event trigger function",
 				"pg_event_trigger_table_rewrite_reason()")));
 
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index 6a113b8..6cc3ed9 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -278,6 +278,7 @@ Section: Class 39 - External Routine Invocation Exception
 39004    E    ERRCODE_E_R_I_E_NULL_VALUE_NOT_ALLOWED                         null_value_not_allowed
 39P01    E    ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED                      trigger_protocol_violated
 39P02    E    ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED                          srf_protocol_violated
+39P03    E    ERRCODE_E_R_I_E_EVENT_TRIGGER_PROTOCOL_VIOLATED                event_trigger_protocol_violated
 
 Section: Class 3B - Savepoint Exception
 
#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#4)
Re: New error code to track unsupported contexts

Pushed this.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Michael Paquier
michael.paquier@gmail.com
In reply to: Alvaro Herrera (#5)
Re: New error code to track unsupported contexts

2015-04-09 3:45 GMT+09:00 Alvaro Herrera <alvherre@2ndquadrant.com>:

Pushed this.

Thanks!
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers