You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "XieJiann (via GitHub)" <gi...@apache.org> on 2023/02/10 02:44:09 UTC
[GitHub] [doris] XieJiann opened a new pull request, #16586: [Nereids](refactor): add two phase sort
XieJiann opened a new pull request, #16586:
URL: https://github.com/apache/doris/pull/16586
# Proposed changes
Issue Number: close #xxx
## 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](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #16586: [refactor](Nereids): add two phase sort
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16586:
URL: https://github.com/apache/doris/pull/16586#issuecomment-1430658846
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow merged pull request #16586: [refactor](Nereids): add two phase sort
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow merged PR #16586:
URL: https://github.com/apache/doris/pull/16586
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] XieJiann commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105756133
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
这个在PhysicalSort那里有说明（https://github.com/apache/doris/pull/16586/files#diff-b3ddfd96772020a76b2fabe35088a6faaf38f9b492ea1ff48b21523c93f0cab6 ），和自动机关系不大，只是为了区分不同阶段下的sort
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] englefly commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105751625
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
how the phases transferred?
and what is the difference between merge and gather?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] englefly commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105751625
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
please add some comments about
1. how the phases transferred.
2. the difference between merge and gather.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] XieJiann commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105756133
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
这个在PhysicalSort那里有说明，和自动机关系不大，只是为了区分不同阶段下的sort
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] englefly commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105766515
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
+ GATHER_SORT("GatherSort"),
Review Comment:
consider following plan
```
sort(gather)
+---scan(T)
```
if T is replica table, do we add an exchange to gather data?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] XieJiann commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105788563
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
+ GATHER_SORT("GatherSort"),
Review Comment:
Do you mean T has only one tablet?
In this case, the scan node has the Gather property. So the Gather requested by the sortNode is satisfied by its child. We don't need to add a gather exchange node
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] XieJiann commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105788563
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
+ GATHER_SORT("GatherSort"),
Review Comment:
Do you mean T has only one tablet?
In this case, the scan node has the Gather property. So the Gather request by the sortNode is satisfied by its child. We don't need to add a gather exchange node
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] englefly commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105751625
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
可否画一个类似自动机的图，说明一下这几个状态的转移关系？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #16586: [refactor](Nereids): add two phase sort
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16586:
URL: https://github.com/apache/doris/pull/16586#issuecomment-1430658823
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] englefly commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105751625
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
how the phase tranformed?
and what is the difference between merge and gather?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
how the phases tranformed?
and what is the difference between merge and gather?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] XieJiann commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105756133
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
Review Comment:
这个在PhysicalSort那里有说明（https://github.com/apache/doris/pull/16586/files#diff-b3ddfd96772020a76b2fabe35088a6faaf38f9b492ea1ff48b21523c93f0cab6），和自动机关系不大，只是为了区分不同阶段下的sort
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] XieJiann commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105788563
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/SortPhase.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans;
+
+/**
+ * Represents different phase of agg and map it to the
+ * enum of agg phase definition of stale optimizer.
+ */
+public enum SortPhase {
+ MERGE_SORT("MergeSort"),
+ GATHER_SORT("GatherSort"),
Review Comment:
Do you mean T has only one tablet?
In this case, the scan node has Gather property. So the gather is satisfied by the child of `sort`. We don't need to add a gather exchange node
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #16586: [refactor](Nereids): add two phase sort
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on code in PR #16586:
URL: https://github.com/apache/doris/pull/16586#discussion_r1105957734
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalTopNToPhysicalTopN.java:
##########
@@ -19,20 +19,33 @@
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.SortPhase;
+import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.trees.plans.physical.PhysicalTopN;
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
/**
* Implementation rule that convert logical top-n to physical top-n.
*/
public class LogicalTopNToPhysicalTopN extends OneImplementationRuleFactory {
@Override
public Rule build() {
- return logicalTopN().then(topN -> new PhysicalTopN<>(
- topN.getOrderKeys(),
- topN.getLimit(),
- topN.getOffset(),
- topN.getLogicalProperties(),
- topN.child())
- ).toRule(RuleType.LOGICAL_TOP_N_TO_PHYSICAL_TOP_N_RULE);
+ return logicalTopN().thenApplyMulti(ctx -> twoPhaseSort(ctx.root))
+ .toRule(RuleType.LOGICAL_TOP_N_TO_PHYSICAL_TOP_N_RULE);
+ }
+
+ private List<PhysicalTopN<? extends Plan>> twoPhaseSort(LogicalTopN logicalTopN) {
+ PhysicalTopN localSort = new PhysicalTopN(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
+ logicalTopN.getOffset(), logicalTopN.getLogicalProperties(), logicalTopN.child(0),
+ SortPhase.LOCAL_SORT);
+ PhysicalTopN twoPhaseSort = new PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
+ logicalTopN.getOffset(), logicalTopN.getLogicalProperties(), localSort, SortPhase.MERGE_SORT);
+ PhysicalTopN onePhaseSort = new PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
+ logicalTopN.getOffset(), logicalTopN.getLogicalProperties(), localSort.child(0), SortPhase.GATHER_SORT);
+ return Lists.newArrayList(twoPhaseSort, onePhaseSort);
Review Comment:
we should only return two sort plan:
- gather sort: require child's distribution is GATHER
- merge sort: first stage require child's distribution is ANY, and second stage require is GATHER
then, cost and enforce could generate right plan for the two type of sort.
i think the main reason u use three types is easy for translation, right?
so, we could have two type of sort.
- LOCAL: translate to sort node
- MERGE: merge sort info into its child exchange node.
and, u could generate enforce require as AggregateStrategy do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org