You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@geaflow.apache.org by "ftljk (via GitHub)" <gi...@apache.org> on 2025/08/14 08:05:19 UTC
[PR] feat: Optimizing Sorting Performance via Radix Sort Algorithm [geaflow]
ftljk opened a new pull request, #589:
URL: https://github.com/apache/geaflow/pull/589
### What changes were proposed in this pull request?
<!--Please describe the major changes for this PR-->
This pull request introduces an radix sorting algorithm implementation tailored for integers and short strings.
### How was this PR tested?
- [ ] Tests have Added for the changes
- [ ] Production environment verified
--
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: issues-unsubscribe@geaflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org
Re: [PR] feat: Optimizing Sorting Performance via Radix Sort Algorithm [geaflow]
Posted by "CLAassistant (via GitHub)" <gi...@apache.org>.
CLAassistant commented on PR #589:
URL: https://github.com/apache/geaflow/pull/589#issuecomment-3187385423
[](https://cla-assistant.io/apache/geaflow?pullRequest=589) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/apache/geaflow?pullRequest=589) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/apache/geaflow?pullRequest=589) it.</sub>
--
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: issues-unsubscribe@geaflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org
Re: [PR] feat: Optimizing Sorting Performance via Radix Sort Algorithm [geaflow]
Posted by "yaozhongq (via GitHub)" <gi...@apache.org>.
yaozhongq commented on code in PR #589:
URL: https://github.com/apache/geaflow/pull/589#discussion_r2336415225
##########
geaflow/geaflow-dsl/geaflow-dsl-runtime/src/main/java/org/apache/geaflow/dsl/runtime/function/table/order/SortInfo.java:
##########
@@ -36,4 +39,21 @@ public SortInfo copy(List<OrderByField> orderByFields) {
sortInfo.fetch = this.fetch;
return sortInfo;
}
+
+ public boolean isRadixSortable() {
+ for (int i = 0; i < this.orderByFields.size(); i++) {
+ OrderByField field = this.orderByFields.get(i);
+ IType<?> orderType = field.expression.getOutputType();
+ if (orderType.getTypeClass() != Integer.class && orderType.getTypeClass() != BinaryString.class) {
+ return false;
+ } else if (orderType.getTypeClass() == BinaryString.class) {
+ int precision = ((BinaryStringType) orderType).getPrecision();
+ // MongoDB ObjectId: 24-character hexadecimal
+ if (precision > 24 || precision < 0) {
+ return false;
+ }
+ }
+ }
+ return true;
Review Comment:
Default is true could be crash?
##########
geaflow/geaflow-dsl/geaflow-dsl-runtime/src/main/java/org/apache/geaflow/dsl/runtime/function/table/OrderByTimSort.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.geaflow.dsl.runtime.function.table;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.geaflow.dsl.common.data.Row;
+import org.apache.geaflow.dsl.common.function.FunctionContext;
+import org.apache.geaflow.dsl.runtime.function.table.order.SortInfo;
+import org.apache.geaflow.dsl.runtime.function.table.order.TopNRowComparator;
+
+public class OrderByTimSort implements OrderByFunction {
+
+ private final SortInfo sortInfo;
+
+ private List<Row> allRows;
+
+ private TopNRowComparator<Row> topNRowComparator;
+
+ public OrderByTimSort(SortInfo sortInfo) {
+ this.sortInfo = sortInfo;
+ }
+
+ @Override
+ public void open(FunctionContext context) {
+ this.topNRowComparator = new TopNRowComparator<>(sortInfo);
+ this.allRows = new ArrayList<>();
+ }
+
+ @Override
+ public void process(Row row) {
+ if (sortInfo.fetch == 0) {
+ return;
+ }
+ allRows.add(row);
+ }
+
+ @Override
+ public Iterable<Row> finish() {
+ List<Row> sortedRows = new ArrayList<>(allRows);
+ sortedRows.sort(topNRowComparator);
Review Comment:
Why sort again?
##########
geaflow/geaflow-dsl/geaflow-dsl-runtime/src/main/java/org/apache/geaflow/dsl/runtime/function/table/order/MultiFieldRadixSort.java:
##########
@@ -0,0 +1,225 @@
+/*
+ * 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.geaflow.dsl.runtime.function.table.order;
+
+import java.util.List;
+import org.apache.geaflow.common.binary.BinaryString;
+import org.apache.geaflow.dsl.common.data.Row;
+
+public class MultiFieldRadixSort {
Review Comment:
Please add unit tests and integration tests
--
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: issues-unsubscribe@geaflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org
Re: [PR] feat: Optimizing Sorting Performance via Radix Sort Algorithm [geaflow]
Posted by "Loognqiang (via GitHub)" <gi...@apache.org>.
Loognqiang commented on code in PR #589:
URL: https://github.com/apache/geaflow/pull/589#discussion_r2282338049
##########
geaflow/geaflow-dsl/geaflow-dsl-parser/src/main/java/org/apache/geaflow/dsl/util/SqlTypeUtil.java:
##########
@@ -129,7 +130,9 @@ public static RelDataType convertToRelType(IType<?> type, boolean isNullable,
default:
if (type.isPrimitive()) {
String sqlTypeName = convertToSqlTypeName(type);
- SqlTypeName typeName = SqlTypeName.valueOf(sqlTypeName);
+ SqlTypeName typeName = Types.getType(type.getTypeClass()) == Types.BINARY_STRING ?
Review Comment:
Check Style failed, operatorWrap: '?' should be on a new line.
--
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: issues-unsubscribe@geaflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org
Re: [PR] feat: Optimizing Sorting Performance via Radix Sort Algorithm [geaflow]
Posted by "yaozhongq (via GitHub)" <gi...@apache.org>.
yaozhongq commented on PR #589:
URL: https://github.com/apache/geaflow/pull/589#issuecomment-3274515251
Please add design reports and performance tests.
--
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: issues-unsubscribe@geaflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org
Re: [PR] feat: Optimizing Sorting Performance via Radix Sort Algorithm [geaflow]
Posted by "yaozhongq (via GitHub)" <gi...@apache.org>.
yaozhongq merged PR #589:
URL: https://github.com/apache/geaflow/pull/589
--
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: issues-unsubscribe@geaflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org