velox icon indicating copy to clipboard operation
velox copied to clipboard

Verify min(date) and max(date) aggregate functions end-to-end

Open mbasmanova opened this issue 2 years ago • 2 comments

Currently, min and max aggregate functions with DATE input types produce DATE intermediate results. We need to double check whether this matches intermediate types expected by the Presto planner and if not update the implementation. We would also like to add an e2e test to ensure min(date) and max(date) work properly.

    @Test
    public void testMinMax()
    {
        // tinyint
        assertQuery("SELECT min(cast(linenumber as tinyint)), max(cast(linenumber as tinyint)) FROM lineitem");
        // smallint
        assertQuery("SELECT min(cast(linenumber as smallint)), max(cast(linenumber as smallint)) FROM lineitem");
        // integer
        assertQuery("SELECT min(linenumber), max(linenumber) FROM lineitem");
        // bigint
        assertQuery("SELECT min(orderkey), max(orderkey) FROM lineitem");
        // real
        assertQuery("SELECT min(cast(quantity as real)), max(cast(quantity as real)) FROM lineitem");
        // double
        assertQuery("SELECT min(quantity), max(quantity) FROM lineitem");
        // timestamp
        assertQuery("SELECT min(from_unixtime(orderkey)), max(from_unixtime(orderkey)) FROM lineitem");
    }

CC: @spershin @pramodsatya

mbasmanova avatar Jul 29 '22 16:07 mbasmanova

also needed for min_by/max_by aggregate functions :)

yvxiang avatar Aug 01 '22 10:08 yvxiang

#2189 added in Velox to address the initialization issue for min(date) aggregate, https://github.com/facebookexternal/presto_cpp/pull/827 adds e2e test in presto_cpp. #2235 added in Velox to fix the initialization error for min_by, max_by aggregates for DATE inputs.

pramodsatya avatar Aug 08 '22 22:08 pramodsatya