IMPALA-5623: Fix lag() on STRING cols to release UDF mem IMPALA-4120 fixed an issue where lead/lag was potentially operating on memory that the UDA didn't own, resulting in potentially wrong results. As part of that fix, lead and lag started allocating 'global' UDF memory (e.g. via Allocate() rather than AllocateLocal()) in Init() which needs to be freed in Serialize() or Finalize(), but only lead() was updated to free the memory. This memory is eventually freed when the fragment is torn down, but as a result of not freeing the memory in Serialize or Finalize, the memory may be allocated longer than necessary.
Change-Id: Id2b69b4ccb9cac076abca19bed6f0b1dd11dfff3 Reviewed-on: http://gerrit.cloudera.org:8080/7371 Reviewed-by: Matthew Jacobs <[email protected]> Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/3939591a Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/3939591a Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/3939591a Branch: refs/heads/master Commit: 3939591aa891ce8e277f9c9a11e155927d413ea1 Parents: a15d8ac Author: Matthew Jacobs <[email protected]> Authored: Thu Jul 6 17:13:03 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Jul 11 02:07:55 2017 +0000 ---------------------------------------------------------------------- .../java/org/apache/impala/catalog/BuiltinsDb.java | 4 +++- .../queries/QueryTest/analytic-fns.test | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3939591a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java b/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java index 58f5d15..3eba678 100644 --- a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java +++ b/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java @@ -1040,7 +1040,9 @@ public class BuiltinsDb extends Db { db, "lag", Lists.newArrayList(t, Type.BIGINT, t), t, t, prefix + OFFSET_FN_INIT_SYMBOL.get(t), prefix + OFFSET_FN_UPDATE_SYMBOL.get(t), - null, null, null)); + null, + t == Type.STRING ? stringValGetValue : null, + t == Type.STRING ? stringValSerializeOrFinalize : null)); db.addBuiltin(AggregateFunction.createAnalyticBuiltin( db, "lead", Lists.newArrayList(t, Type.BIGINT, t), t, t, prefix + OFFSET_FN_INIT_SYMBOL.get(t), http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3939591a/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test b/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test index 88339e6..34b5196 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test +++ b/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test @@ -1904,6 +1904,22 @@ from functional.alltypes) x where x.a = x.b ---- TYPES BIGINT +---- ERRORS +---- RESULTS +7299 +==== +---- QUERY +# Regression test for IMPALA-5623: lag() should free UDF allocated memory. +# i.e. the fix for IMPALA-4120 should apply to lag() as well. +select count(*) from ( +select + from_unixtime(lag(bigint_col, 1) over (order by id), 'yyyyMMddHH:mm:ss') as a, + lag(from_unixtime(bigint_col, 'yyyyMMddHH:mm:ss'), 1) over (order by id) AS b +from functional.alltypes) x +where x.a = x.b +---- TYPES +BIGINT +---- ERRORS ---- RESULTS 7299 ====
