Tighten checks for "bins" elements in width_bucket(x, bins)

spershin commented 1 week ago

Description

Make width_bucket(x, bins) throw error if it finds a null or non-finite element in bins.

Motivation and Context

Solves https://github.com/prestodb/presto/issues/24055

Impact

Changes behavior of width_bucket(x, bins) which previously was treating all null elements in bins as zero. Now the function will throw an error.

Test Plan

Updated unit test to handle few cases with nulls.

Contributor checklist

[ ] Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
[ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
[ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality.
[ ] If release notes are required, they follow the release notes guidelines.
[ ] Adequate tests were added if applicable.
[ ] CI passed.

Release Notes

== RELEASE NOTES ==
General Changes
* Fix behavior of :func:`width_bucket(x, bins) -> bigint` which previously treated all ``null`` elements in bins as ``0``. Now the function will throw an error if it finds a ``null`` or non-finite element in ``bins``..  :pr:`24103`

steveburnett commented 6 days ago

Because this PR changes user-facing behavior, I think we should consider a release note entry for this PR. Perhaps something like:

== RELEASE NOTES ==
General Changes
* Fix behavior of :func:`width_bucket(x, bins) -> bigint` which previously treated all ``null`` elements in bins as ``0``. Now the function will throw an error.  :pr:`24103`

(I did a local doc build to verify that :func:width_bucket(x, bins) -> bigint generates a working link from a file in /release/ to this function in functions/math.rst.)

prestodb-ci commented 6 days ago

Saved that user @spershin is from Meta

steveburnett commented 5 days ago

Nit on the formatting of the release note entry - please add a row of three ` above and below the release note block. Like this:

== RELEASE NOTES ==

General Changes
* Fix behavior of :func:`width_bucket(x, bins) -> bigint` which previously treated all ``null`` elements in bins as ``0``. Now the function will throw an error if it finds a ``null`` or non-finite element in ``bins``..  :pr:`24103`

prestodb / presto