Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

TAJO-1954: Fix memory leak in physical operator.#838

Closed
jinossy wants to merge 3 commits into
apache:masterfrom
jinossy:TAJO-1954
Closed

TAJO-1954: Fix memory leak in physical operator.#838
jinossy wants to merge 3 commits into
apache:masterfrom
jinossy:TAJO-1954

Conversation

@jinossy
Copy link
Copy Markdown
Member

@jinossy jinossy commented Nov 2, 2015

No description provided.

@jihoonson
Copy link
Copy Markdown
Contributor

+1 LGTM!

@asfgit asfgit closed this in f4abd4e Nov 2, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinossy I have a question? Do we need check null check for aggregateExecs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charsyam
I refer to close() but It seems to the unnecessary checking. Are you working on this operator ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinossy I'm sorry.
I tested below code. and it causes NPE.

   List<String> lists = null;
    for(String a : lists) {
    }

I think null checking is needed. sorry to disturb you.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PhysicalPlannerImpl.createSortAggregationDistinctGroupbyExec always set the array, so it can not occurs NPE.
Next time, I will refactor these code.

Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants