Check consistency of param N for min_byN/max_byN

[mmorgan98]

Description

Fixes issue #20920 to check for uniqueness/consistency of N for maxbyN/minbyN aggregate functions.

Motivation and Context

Currently the uniqueness/consistency of param N of minbyN/maxbyN is not checked.

Impact

The test case in issue #20920 now fails as expected:

presto> SELECT max_by(c0, c1, c2)
     ->       FROM (
     ->         VALUES
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '0', BIGINT '10', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '1', BIGINT '9', BIGINT '3'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '2', BIGINT '8', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '3', BIGINT '7', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '4', BIGINT '6', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '5', BIGINT '5', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '6', BIGINT '4', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '7', BIGINT '3', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '8', BIGINT '2', BIGINT '2'),
     ->           (cast(NULL as BIGINT), cast(NULL as BIGINT), cast(NULL as BIGINT)),
     ->           (BIGINT '9', BIGINT '1', BIGINT '2')
     ->       ) AS tmp (c0, c1, c2);

Query 20231103_190942_00004_r6sfw, FAILED, 1 node
Splits: 1 total, 0 done (0.00%)
[Latency: client-side: 97ms, server-side: 68ms] [0 rows, 0B] [0 rows/s, 0B/s]

Query 20231103_190942_00004_r6sfw failed: Count argument is not constant: found multiple values [3, 2]

Test Plan

Added UT testInconsistentN() inside /presto-main/src/test/java/com/facebook/presto/operator/aggregation/minmaxby/TestMinMaxByNAggregation.java

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

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* minbyN/maxbyN's third argument (N) is now checked to be unique.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mmorgan98 / name: Matthew Morgan (e304fa9)

[mmorgan98]

Signed CLA

[tdcmeehan]

LGTM % nit on the test

[tdcmeehan]

Suggested change

	@Test
	public void testInconsistentN()
	{
	JavaAggregationFunctionImplementation function = getMaxByAggregation(BIGINT, BIGINT, BIGINT);
	try {
	groupedAggregation(function, new Page(createDoublesBlock(new Double[] {3.0, 2.0, 1.0}), createDoublesBlock(new Double[] {1.0, 2.0, 3.0}), createLongsBlock(new Long[] {2L, 2L, 3L})));
	}
	catch (PrestoException e) {
	assertEquals(e.getMessage(), "Count argument is not constant: found multiple values [3, 2]");
	}
	}
	@Test(expectedExceptions = PrestoException.class, expectedExceptionsMessageRegExp = "Count argument is not constant: found multiple values [3, 2")
	public void testInconsistentN()
	{
	JavaAggregationFunctionImplementation function = getMaxByAggregation(BIGINT, BIGINT, BIGINT);
	groupedAggregation(function, new Page(createDoublesBlock(new Double[] {3.0, 2.0, 1.0}), createDoublesBlock(new Double[] {1.0, 2.0, 3.0}), createLongsBlock(new Long[] {2L, 2L, 3L})));
	}

[mmorgan98]

See latest commit

[tdcmeehan]

CC: @xumingming

[xumingming]

Thanks @mmorgan1230 @tdcmeehan

[mmorgan98]

FYI - had to reset back 1 commit as I had the incorrect local git config setup. I recommitted the same content as the initial commit that was reviewed and will update the test with a follow up commit.

[tdcmeehan]

LGTM! Please squash commits and I can approve.