Обсуждение: pgsql: Remove unnecessary uses of Abs()
Remove unnecessary uses of Abs() Use C standard abs() or fabs() instead. Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/f14aad5169baa5e2ac25d49f1d18f9d5cb3bc7f2 Modified Files -------------- contrib/btree_gist/btree_date.c | 4 ++-- contrib/btree_gist/btree_float8.c | 4 ++-- contrib/btree_gist/btree_int2.c | 2 +- contrib/btree_gist/btree_int4.c | 2 +- contrib/btree_gist/btree_interval.c | 2 +- contrib/btree_gist/btree_time.c | 2 +- contrib/btree_gist/btree_ts.c | 2 +- contrib/btree_gist/btree_utils_num.h | 2 +- contrib/btree_gist/btree_utils_var.c | 2 +- contrib/cube/cube.c | 2 +- contrib/intarray/_int_gist.c | 3 ++- contrib/intarray/_intbig_gist.c | 4 +++- contrib/ltree/_ltree_gist.c | 4 +++- contrib/seg/seg.c | 4 ++-- src/backend/access/gist/gistproc.c | 4 ++-- src/backend/optimizer/geqo/geqo_erx.c | 8 ++++---- src/backend/partitioning/partbounds.c | 4 ++-- src/backend/utils/adt/datetime.c | 8 ++++---- src/backend/utils/adt/numeric.c | 20 ++++++++++---------- src/backend/utils/adt/rangetypes_gist.c | 4 ++-- src/backend/utils/adt/selfuncs.c | 2 +- src/backend/utils/adt/timestamp.c | 4 ++-- src/backend/utils/adt/tsgistidx.c | 2 +- src/backend/utils/adt/tsrank.c | 2 +- src/backend/utils/misc/guc.c | 2 +- src/interfaces/ecpg/pgtypeslib/interval.c | 4 ++-- 26 files changed, 54 insertions(+), 49 deletions(-)
Peter Eisentraut <peter@eisentraut.org> writes:
> Remove unnecessary uses of Abs()
Re-reading this, I noticed something that's probably not good:
in ecpg/pgtypeslib/interval.c you did
- sprintf(cp, "%02d.%0*d", abs(sec), precision, (int) Abs(fsec));
+ sprintf(cp, "%02d.%0*d", abs(sec), precision, abs(fsec));
(in 2 places). I think this has possibly broken the direction of rounding
for negative values, because we'll now truncate to integer before changing
sign. Maybe it's okay but you have to make assumptions about what it'll
do. Safer would have been
+ sprintf(cp, "%02d.%0*d", abs(sec), precision, (int) fabs(fsec));
There is similar-looking coding in remove_gene(), but I didn't check
the data type involved.
regards, tom lane
On 07.10.22 16:38, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> Remove unnecessary uses of Abs() > > Re-reading this, I noticed something that's probably not good: > in ecpg/pgtypeslib/interval.c you did > > - sprintf(cp, "%02d.%0*d", abs(sec), precision, (int) Abs(fsec)); > + sprintf(cp, "%02d.%0*d", abs(sec), precision, abs(fsec)); > > (in 2 places). I think this has possibly broken the direction of rounding > for negative values, because we'll now truncate to integer before changing > sign. Maybe it's okay but you have to make assumptions about what it'll > do. Safer would have been > > + sprintf(cp, "%02d.%0*d", abs(sec), precision, (int) fabs(fsec)); fsec is of type fsec_t, which is int32. So these expressions are integer all the way through. > There is similar-looking coding in remove_gene(), but I didn't check > the data type involved. There, the type "Gene" is int, so also no floats involved AFAICT.
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> fsec is of type fsec_t, which is int32. So these expressions are
> integer all the way through.
Ah, okay. My instincts were leftover from a time when it could
have been float8. Sorry for the noise.
regards, tom lane