Properly mark NULL returns in numeric aggregates

From: Jesse Zhang <sbjesse(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Cc: Denis Smirnov <sd(at)arenadata(dot)io>, Soumyadeep Chakraborty <sochakraborty(at)pivotal(dot)io>
Subject: Properly mark NULL returns in numeric aggregates
Date: 2020-04-09 23:22:11
Message-ID: CAGf+fX4ZB9=5eUaHdxUWA7au8KOd4Mychc-jzW5htbAAGFXGWw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi hackers,

We found that several functions -- namely numeric_combine,
numeric_avg_combine, numeric_poly_combine, and int8_avg_combine -- are
returning NULL without signaling the nullity of datum in fcinfo.isnull.
This is obscured by the fact that the only functions in core (finalfunc
for various aggregates) that those return values feed into happen to
tolerate (or rather, not quite distinguish) zero-but-not-NULL trans
values.

In Greenplum, this behavior becomes problematic because Greenplum
serializes internal trans values before spilling the hash table. The
serial functions (numeric_serialize and friends) are strict functions
that will blow up when they are given null (either in the C sense or the
SQL sense) inputs.

In Postgres if we change hash aggregation in the future to spill the
hash table (vis-à-vis the input tuples), this issues would manifest
itself in the final aggregate because we'll serialize the combined (and
likely incorrectly null) trans values.

Please find attached a small patch fixing said issue. Originally
reported by Denis Smirnov over at
https://github.com/greenplum-db/gpdb/pull/9878

Cheers,
Jesse and Deep

Attachment Content-Type Size
0001-Properly-mark-NULL-returns-in-numeric-aggregates.patch text/x-patch 2.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-04-09 23:30:19 Re: more ALTER .. DEPENDS ON EXTENSION fixes
Previous Message Michail Nikolaev 2020-04-09 23:02:44 Re: Thoughts on "killed tuples" index hint bits support on standby