You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2017/07/11 03:37:49 UTC
[5/6] incubator-impala git commit: IMPALA-5623: Fix lag() on STRING
cols to release UDF mem
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 <mj...@cloudera.com>
Reviewed-by: Dan Hecht <dh...@cloudera.com>
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 <mj...@cloudera.com>
Authored: Thu Jul 6 17:13:03 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
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
====