fujii.y...@df.mitsubishielectric.co.jp писал 2023-06-06 06:08:
Hi Mr.Pyhalov.
Thank you for your always thoughtful review.
From: Alexander Pyhalov <a.pyha...@postgrespro.ru>
Sent: Monday, June 5, 2023 6:00 PM
Have found one issue -
src/backend/catalog/pg_aggregate.c
585 if(strcmp(strVal(linitial(aggpartialfnName)),
aggName) == 0){
586 if(((aggTransType != INTERNALOID) &&
(finalfn != InvalidOid))
587 || ((aggTransType ==
INTERNALOID) && (finalfn != serialfn)))
588 elog(ERROR, "%s is not its own
aggpartialfunc", aggName);
589 } else {
Here string comparison of aggName and aggpartialfnName looks very
suspicios - it seems you should compare oids, not names (in this case,
likely oids of transition function and partial aggregation function).
The fact that aggregate name matches partial aggregation function name
is not a enough to make any decisions.
I see no problem with this string comparison. Here is the reason.
The intent of this code is, to determine whether the user specifies
the new aggregate function whose aggpartialfunc is itself.
For two aggregate functions,
If the argument list and function name match, then the two aggregate
functions must match.
By definition of aggpartialfunc,
every aggregate function and its aggpartialfn must have the same
argument list.
Thus, if aggpartialfnName and aggName are equal as strings,
I think it is correct to determine that the user is specifying
the new aggregate function whose aggpartialfunc is itself.
However, since the document does not state these intentions
I think your suspicions are valid.
Therefore, I have added a specification to the document reflecting the
above intentions.
Hi. Let me explain.
Look at this example, taken from test.
CREATE AGGREGATE udf_avg_p_int4(int4) (
sfunc = int4_avg_accum,
stype = _int8,
combinefunc = int4_avg_combine,
initcond = '{0,0}'
);
CREATE AGGREGATE udf_sum(int4) (
sfunc = int4_avg_accum,
stype = _int8,
finalfunc = int8_avg,
combinefunc = int4_avg_combine,
initcond = '{0,0}',
aggpartialfunc = udf_avg_p_int4
);
Now, let's create another aggregate.
# create schema test ;
create aggregate test.udf_avg_p_int4(int4) (
sfunc = int4_avg_accum,
stype = _int8,
finalfunc = int8_avg,
combinefunc = int4_avg_combine,
initcond = '{0,0}',
aggpartialfunc = udf_avg_p_int4
);
ERROR: udf_avg_p_int4 is not its own aggpartialfunc
What's the difference between test.udf_avg_p_int4(int4) aggregate and
udf_sum(int4)? They are essentially the same, but second one can't be
defined.
Also note difference:
# CREATE AGGREGATE udf_sum(int4) (
sfunc = int4_avg_accum,
stype = _int8,
finalfunc = int8_avg,
combinefunc = pg_catalog.int4_avg_combine,
initcond = '{0,0}',
aggpartialfunc = udf_avg_p_int4
);
CREATE AGGREGATE
# CREATE AGGREGATE udf_sum(int4) (
sfunc = int4_avg_accum,
stype = _int8,
finalfunc = int8_avg,
combinefunc = pg_catalog.int4_avg_combine,
initcond = '{0,0}',
aggpartialfunc = public.udf_avg_p_int4
);
ERROR: aggpartialfnName is invalid
It seems that assumption about aggpartialfnName - that it's a
non-qualified name is incorrect. And if we use qualified names, we can't
compare just names, likely we should compare oids.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
