Skip to content

[Refactor](Nereids) Simplify get input and output slots for plan/expression.#12356

Merged
924060929 merged 1 commit intoapache:masterfrom
wangshuo128:input_slot
Sep 7, 2022
Merged

[Refactor](Nereids) Simplify get input and output slots for plan/expression.#12356
924060929 merged 1 commit intoapache:masterfrom
wangshuo128:input_slot

Conversation

@wangshuo128
Copy link
Copy Markdown
Contributor

@wangshuo128 wangshuo128 commented Sep 5, 2022

Proposed changes

Simplify the code of getting input/output slots from Expression or Plan.

new interfaces add

Expression:
getInputSlots: Get all the input slots of the expression.

Plan:

  • getOutputSet: Get the output slot set of the plan.
  • getInputSlots: Get the input slot set of the plan.

changed interface

TreeNode:

  • collect: return set as result instead of list.

Problem summary

Describe your changes.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@wangshuo128 wangshuo128 force-pushed the input_slot branch 2 times, most recently from b82bb27 to 5c115ec Compare September 6, 2022 12:30
morrySnow
morrySnow previously approved these changes Sep 7, 2022
Copy link
Copy Markdown
Contributor

@morrySnow morrySnow left a comment

Choose a reason for hiding this comment

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

LGTM

@924060929 924060929 merged commit f2923f9 into apache:master Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants