You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues-all@impala.apache.org by "ASF subversion and git services (Jira)" <ji...@apache.org> on 2025/03/05 13:58:00 UTC
[jira] [Commented] (IMPALA-13793) Always close expressions
[ https://issues.apache.org/jira/browse/IMPALA-13793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932640#comment-17932640 ]
ASF subversion and git services commented on IMPALA-13793:
----------------------------------------------------------
Commit 24179d46cce2a1ff063f9ae37f0291f2dd23d8ed in impala's branch refs/heads/master from Daniel Becker
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=24179d46c ]
IMPALA-13793: Always close expressions
The Expr class has a Close() method, which is currently only used for
releasing 'cache_entry_', which is used for UDF or UDAF functions. If
Close() is not called for an Expr that uses 'cache_entry_', a DCHECK
fails in the destructor of Expr. However, if an Expr does not use
'cache_entry_' is not closed, nothing happens.
Classes holding Expr objects should always close
expressions because they may use 'cache_entry_'. However, if tests fail
to include cases where 'cache_entry_' is used, bugs may not be detected:
one such case was IMPALA-13770.
In debug builds, this change adds a new field, 'closed_', to the Expr
class, which is set to true in Close(). A DCHECK in the destructor fires
if Close() has not been called, regardless of whether 'cache_entry_' is
used. This makes it easier to detect cases where expressions are not
closed.
To avoid increasing the size of expressions, the 'closed_' field is not
added in release mode.
Existing cases where expressions are not closed are fixed in this
change.
Testing:
- tests are updated to close expressions
Change-Id: I9784652f5e135f1f8cd7f0fa6df471e64d27c477
Reviewed-on: http://gerrit.cloudera.org:8080/22528
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
> Always close expressions
> ------------------------
>
> Key: IMPALA-13793
> URL: https://issues.apache.org/jira/browse/IMPALA-13793
> Project: IMPALA
> Issue Type: Improvement
> Components: Backend
> Reporter: Daniel Becker
> Assignee: Daniel Becker
> Priority: Major
>
> The Expr class has a Close() method, which is currently only used for releasing 'cache_entry_', which is used for UDF or UDAF functions. If Close() is not called for an Expr that uses 'cache_entry_', a DCHECK fails in the destructor of Expr.
> However, if an Expr does not use 'cache_entry_' is not closed, nothing happens. Classes holding Expr objects should always close expressions because they may use 'cache_entry_'. However, if tests fail to include cases where 'cache_entry_' is used, bugs may not be detected: one such case was [IMPALA-13770|http://issues.apache.org/jira/browse/IMPALA-13770].
> We should always enforce closing expressions to make it easier to detect bugs.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org