moving aggregate bad error message

Started by Jeff Janesover 5 years ago2 messages
#1Jeff Janes
jeff.janes@gmail.com
2 attachment(s)

I was wondering if I could just add minvfunc, and have the rest of the m*
functions be assumed to be the same as their non-moving counterparts.
Apparently the answer is 'no'. But in the process, I found a bad error
message. When omitting mfinalfunc when there is a finalfunc, I get the
error:

"ERROR: moving-aggregate implementation returns type jj_state, but plain
implementation returns type jj_state." A rather peculiar
complaint, analogous to the infamous "something failed: Success".

Looking at the code, it seems we are testing rettype != finaltype, but
reporting aggmTransType and aggTransType. Why aren't we reporting what we
are testing?

With the attached patch, it gives the more sensible "ERROR:
moving-aggregate implementation returns type jj_state, but plain
implementation returns type numeric."

Cheers,

Jeff

Attachments:

jjagg.sqltext/plain; charset=US-ASCII; name=jjagg.sqlDownload
moving_agg_error.patchapplication/octet-stream; name=moving_agg_error.patchDownload
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index a0554f0d79..0cf1da6ebb 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -565,8 +565,8 @@ AggregateCreate(const char *aggName,
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
 					 errmsg("moving-aggregate implementation returns type %s, but plain implementation returns type %s",
-							format_type_be(aggmTransType),
-							format_type_be(aggTransType))));
+							format_type_be(rettype),
+							format_type_be(finaltype))));
 	}
 
 	/* handle sortop, if supplied */
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jeff Janes (#1)
Re: moving aggregate bad error message

Jeff Janes <jeff.janes@gmail.com> writes:

Looking at the code, it seems we are testing rettype != finaltype, but
reporting aggmTransType and aggTransType. Why aren't we reporting what we
are testing?

Silly thinko, apparently. Your fix looks right, will push.

regards, tom lane