Propagate segment errcodes to dispatcher

The errcode thrown in an ereport() on a segment was passed back to
the dispatcher, but then dropped and replaced with a default errcode
of ERRCODE_DATA_EXCEPTION. This works for most situations, but when
trapping errors the exact errcode must be propagated. This extends
the API to extract the errcode as well. The below case illustrates
the previous issue:

  CREATE TABLE test1(id int primary key);
  CREATE TABLE test2(id int primary key);
  INSERT INTO test1 VALUES(1);
  INSERT INTO test2 VALUES(1);
  CREATE OR REPLACE FUNCTION merge_table() RETURNS void AS $$
  DECLARE
	v_insert_sql varchar;
  BEGIN
	v_insert_sql :='INSERT INTO test1 SELECT * FROM test2';
	EXECUTE v_insert_sql;
	EXCEPTION WHEN unique_violation THEN
		RAISE NOTICE 'unique_violation';
	END;
  $$ LANGUAGE plpgsql volatile;
  SELECT merge_table();

src/backend/cdb/dispatcher/cdbdisp.c

@@ -170,15 +170,25 @@ CdbCheckDispatchResult(struct CdbDispatcherState *ds,
  *
  * Return Values:
  *   Return NULL If one or more QEs got Error in which case qeErrorMsg contain
- *   QE error messages.
+ *   QE error messages and qeErrorCode the ERRCODE thrown.
  */
 struct CdbDispatchResults *
-cdbdisp_getDispatchResults(struct CdbDispatcherState *ds, StringInfoData *qeErrorMsg)
+cdbdisp_getDispatchResults(struct CdbDispatcherState *ds,
+						   StringInfoData *qeErrorMsg, int *qeErrorCode)
 {
 	int			errorcode;
 
 	if (!ds || !ds->primaryResults)
+	{
+		/*
+		 * Fallback in case we have no dispatcher state.  Since the caller is
+		 * likely to output qeErrorMsg on NULL return, add an error message to
+		 * aid debugging.
+		 */
+		*qeErrorCode = ERRCODE_INTERNAL_ERROR;
+		appendStringInfoString(qeErrorMsg, "dispatcher error");
 		return NULL;
+	}
 
 	/* wait QEs to return results or report errors */
 	CdbCheckDispatchResult(ds, DISPATCH_WAIT_NONE);
@@ -188,6 +198,8 @@ cdbdisp_getDispatchResults(struct CdbDispatcherState *ds, StringInfoData *qeErro
 
 	if (errorcode)
 	{
+		if (qeErrorCode)
+			*qeErrorCode = errorcode;
 		cdbdisp_dumpDispatchResults(ds->primaryResults, qeErrorMsg);
 		return NULL;
 	}
@@ -206,7 +218,8 @@ cdbdisp_getDispatchResults(struct CdbDispatcherState *ds, StringInfoData *qeErro
 void
 cdbdisp_finishCommand(struct CdbDispatcherState *ds)
 {
-	StringInfoData qeErrorMsg = {NULL, 0, 0, 0};
+	StringInfoData		qeErrorMsg = {NULL, 0, 0, 0};
+	int					qeErrorCode = 0;
 	CdbDispatchResults *pr = NULL;
 
 	/*
@@ -234,17 +247,12 @@ cdbdisp_finishCommand(struct CdbDispatcherState *ds)
 	{
 		initStringInfo(&qeErrorMsg);
 
-		pr = cdbdisp_getDispatchResults(ds, &qeErrorMsg);
+		pr = cdbdisp_getDispatchResults(ds, &qeErrorMsg, &qeErrorCode);
 
 		if (!pr)
 		{
-			/*
-			 * XXX: It would be nice to get more details from the segment, not
-			 * just the error message. In particular, an error code would be
-			 * nice. DATA_EXCEPTION is a pretty wild guess on the real cause.
-			 */
 			ereport(ERROR,
-					(errcode(ERRCODE_DATA_EXCEPTION),
+					(errcode(qeErrorCode),
 					 errmsg("%s", qeErrorMsg.data)));
 		}
 

src/backend/cdb/dispatcher/cdbdisp_dtx.c

@@ -143,7 +143,7 @@ CdbDispatchDtxProtocolCommand(DtxProtocolCommand dtxProtocolCommand,
 		/*
 		 * Wait for all QEs to finish.	Don't cancel.
 		 */
-		pr = cdbdisp_getDispatchResults(&ds, errmsgbuf);
+		pr = cdbdisp_getDispatchResults(&ds, errmsgbuf, NULL);
 
 		if (!GangOK(primaryGang))
 		{

src/backend/cdb/dispatcher/cdbdisp_query.c

@@ -439,7 +439,8 @@ cdbdisp_dispatchCommandInternal(const char *strCommand,
 {
 	struct CdbDispatcherState ds = {NULL, NULL, NULL};
 	CdbDispatchResults *dispatchresults = NULL;
-	StringInfoData qeErrorMsg;
+	StringInfoData	qeErrorMsg;
+	int				qeErrorCode = 0;
 
 	DispatchCommandQueryParms *pQueryParms;
 	Gang	   *primaryGang;
@@ -519,29 +520,21 @@ cdbdisp_dispatchCommandInternal(const char *strCommand,
 		 * Block until valid results is available or one or more QEs got
 		 * errors.
 		 */
-		dispatchresults = cdbdisp_getDispatchResults(&ds, &qeErrorMsg);
+		dispatchresults = cdbdisp_getDispatchResults(&ds, &qeErrorMsg,
+													 &qeErrorCode);
 
 		/*
 		 * If QEs have errors, throw it up
 		 */
 		if (!dispatchresults)
 		{
-			/*
-			 * debug_string_query is not meaningful for utility statement
-			 */
-			/*
-			 * XXX: It would be nice to get more details from the segment, not
-			 * just the error message. In particular, an error code would be
-			 * nice. DATA_EXCEPTION is a pretty wild guess on the real cause.
-			 */
 			if (serializedQuerytree != NULL)
 				ereport(ERROR,
-						(errcode(ERRCODE_DATA_EXCEPTION),
+						(errcode(qeErrorCode),
 						 errmsg("%s", qeErrorMsg.data)));
 			else
-
 				ereport(ERROR,
-						(errcode(ERRCODE_DATA_EXCEPTION),
+						(errcode(qeErrorCode),
 						 errmsg("could not execute command on QE"),
 						 errdetail("%s", qeErrorMsg.data),
 						 errhint("command: '%s'", strCommand)));

src/include/cdb/cdbdisp.h

@@ -129,10 +129,11 @@ CdbCheckDispatchResult(struct CdbDispatcherState *ds, DispatchWaitMode waitMode)
  *
  * Return Values:
  *   Return NULL If one or more QEs got Error in which case qeErrorMsg contain
- *   QE error messages.
+ *   QE error messages and qeErrorCode the thrown ERRCODE.
  */
 struct CdbDispatchResults *
-cdbdisp_getDispatchResults(struct CdbDispatcherState *ds, StringInfoData *qeErrorMsg);
+cdbdisp_getDispatchResults(struct CdbDispatcherState *ds,
+						   StringInfoData *qeErrorMsg, int *qeErrorCode);
 
 /*
  * Wait for all QEs to finish, then report any errors from the given

src/test/regress/expected/function_extensions.out

@@ -386,3 +386,21 @@ alter function test_srf() EXECUTE ON ANY;
 
 DROP FUNCTION test_srf();
 DROP ROLE srftestuser;
+-- Test error propagation from segments
+CREATE TABLE uniq_test(id int primary key);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "uniq_test_pkey" for table "uniq_test"
+CREATE OR REPLACE FUNCTION trigger_unique() RETURNS void AS $$
+BEGIN
+	INSERT INTO uniq_test VALUES (1);
+	INSERT INTO uniq_test VALUES (1);
+	EXCEPTION WHEN unique_violation THEN
+		raise notice 'unique_violation';
+END;
+$$ LANGUAGE plpgsql volatile;
+SELECT trigger_unique();
+NOTICE:  unique_violation
+ trigger_unique 
+----------------
+ 
+(1 row)
+

src/test/regress/expected/function_extensions_optimizer.out

@@ -387,3 +387,21 @@ alter function test_srf() EXECUTE ON ANY;
 
 DROP FUNCTION test_srf();
 DROP ROLE srftestuser;
+-- Test error propagation from segments
+CREATE TABLE uniq_test(id int primary key);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "uniq_test_pkey" for table "uniq_test"
+CREATE OR REPLACE FUNCTION trigger_unique() RETURNS void AS $$
+BEGIN
+	INSERT INTO uniq_test VALUES (1);
+	INSERT INTO uniq_test VALUES (1);
+	EXCEPTION WHEN unique_violation THEN
+		raise notice 'unique_violation';
+END;
+$$ LANGUAGE plpgsql volatile;
+SELECT trigger_unique();
+NOTICE:  unique_violation
+ trigger_unique 
+----------------
+ 
+(1 row)
+

src/test/regress/sql/function_extensions.sql

@@ -220,3 +220,15 @@ alter function test_srf() EXECUTE ON ANY;
 
 DROP FUNCTION test_srf();
 DROP ROLE srftestuser;
+
+-- Test error propagation from segments
+CREATE TABLE uniq_test(id int primary key);
+CREATE OR REPLACE FUNCTION trigger_unique() RETURNS void AS $$
+BEGIN
+	INSERT INTO uniq_test VALUES (1);
+	INSERT INTO uniq_test VALUES (1);
+	EXCEPTION WHEN unique_violation THEN
+		raise notice 'unique_violation';
+END;
+$$ LANGUAGE plpgsql volatile;
+SELECT trigger_unique();