Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Replace expression unique_ptr with raw pointer #1140

Merged
merged 7 commits into from
Jun 29, 2021
Merged

Replace expression unique_ptr with raw pointer #1140

merged 7 commits into from
Jun 29, 2021

Conversation

Aiee
Copy link
Contributor

@Aiee Aiee commented Jun 16, 2021 •
edited

  1. Replace all unique_ptr(s) with raw pointers in expression implementation and manage object lifetime using ObjectPool.
  2. Disable default expression constructors to preventing mixing raw pointer and unique_ptr.
  3. Disable copy constructor and assignment for expression class to force all expression copy to be done by clone()

Construct an expression: auto* expr = ConstantExpression::make(pool, "Tim Duncan")
Construct an expression with multiple potential kinds: auto* expr = RelationalExpression::makeGT(pool, expr1, expr2)

Depends on vesoft-inc/nebula-common#546 and vesoft-inc/nebula-storage#481

Close #1054

@Aiee Aiee added enhancement Type: make the code neat or more efficient wip Solution: work in progress depend on storage PR: this PR depends on PRs in the storage repo do not review PR: not ready for the code review yet depend on common PR: this PR depends on PRs in the common repo labels Jun 16, 2021
@CLAassistant
Copy link

CLAassistant commented Jun 17, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@Aiee Aiee force-pushed the refactor-pointer-usage branch 2 times, most recently from 57c25b1 to e599a3a Compare June 23, 2021 12:42
@Aiee Aiee changed the title [WIP] Replace expression unique_ptr with raw pointer Replace expression unique_ptr with raw pointer Jun 23, 2021
@Aiee Aiee removed do not review PR: not ready for the code review yet wip Solution: work in progress labels Jun 23, 2021
@Aiee Aiee requested a review from a team June 24, 2021 02:12
yixinglu
yixinglu reviewed Jun 25, 2021
View reviewed changes
src/executor/test/JoinTest.cpp Outdated Show resolved Hide resolved
src/parser/Clauses.h Outdated Show resolved Hide resolved
src/parser/MutateSentences.h Outdated Show resolved Hide resolved
CPWstatic
CPWstatic previously approved these changes Jun 29, 2021
View reviewed changes
jievince
jievince previously approved these changes Jun 29, 2021
View reviewed changes
Copy link
Contributor

@jievince jievince left a comment

Choose a reason for hiding this comment

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

LGTM

yixinglu
yixinglu previously approved these changes Jun 29, 2021
View reviewed changes
@Aiee Aiee added the ready-for-testing PR: ready for the CI test label Jun 29, 2021
@Aiee Aiee dismissed stale reviews from yixinglu, jievince, and CPWstatic via dcab165 June 29, 2021 07:41
@CPWstatic CPWstatic merged commit 43dad92 into vesoft-inc:master Jun 29, 2021
@Aiee Aiee deleted the refactor-pointer-usage branch June 29, 2021 08:32
@jamieliu1023 jamieliu1023 mentioned this pull request Jul 3, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@CPWstatic CPWstatic CPWstatic approved these changes

@jievince jievince jievince approved these changes

@yixinglu yixinglu Awaiting requested review from yixinglu

@nevermore3 nevermore3 Awaiting requested review from nevermore3

@czpmango czpmango Awaiting requested review from czpmango

Assignees
No one assigned
Labels
depend on common PR: this PR depends on PRs in the common repo depend on storage PR: this PR depends on PRs in the storage repo enhancement Type: make the code neat or more efficient ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not mix the smart pointer with raw pointer in AST.
5 participants
@Aiee @CLAassistant @yixinglu @CPWstatic @jievince