lowka commented on code in PR #6140:
URL: https://github.com/apache/ignite-3/pull/6140#discussion_r2175011846
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortExchangeTransposeRule.java:
##########
@@ -60,23 +71,76 @@ private static boolean hashAlike(IgniteDistribution
distribution) {
public void onMatch(RelOptRuleCall call) {
IgniteSort sort = call.rel(0);
- assert sort.isEnforcer() : "Non enforcer sort can not be pushed under
Exchanger";
-
IgniteExchange exchange = call.rel(1);
RelOptCluster cluster = sort.getCluster();
RelCollation collation = sort.collation();
RelNode input = exchange.getInput();
-
- call.transformTo(
- new IgniteExchange(
- cluster,
- exchange.getTraitSet()
- .replace(collation),
- convert(input, input.getTraitSet().replace(collation)),
- exchange.distribution()
- )
+ RelNode newSort = new IgniteSort(
+ cluster,
+ input.getTraitSet().replace(sort.getCollation()),
+ input,
+ sort.getCollation(),
+ null,
+ createLimitForSort(cluster.getPlanner().getExecutor(),
cluster.getRexBuilder(), sort.offset, sort.fetch)
);
+
+ if (sort.offset != null || sort.fetch != null) {
+ call.transformTo(new IgniteLimit(
+ cluster,
+ exchange.getTraitSet()
+ .replace(collation),
+ convert(newSort,
exchange.getTraitSet().replace(collation)),
+ sort.offset,
+ sort.fetch
+ ));
+ } else {
+ call.transformTo(new IgniteExchange(
+ cluster,
+ exchange.getTraitSet()
+ .replace(collation),
+ newSort,
+ exchange.distribution()
+ ), Map.of(newSort, sort));
+ }
+ }
+
+ private static @Nullable RexNode createLimitForSort(
+ @Nullable RexExecutor executor, RexBuilder builder, @Nullable
RexNode offset, @Nullable RexNode fetch
+ ) {
+ if (fetch == null) {
+ // Current implementation of SortNode cannot handle offset-only
case.
+ return null;
+ }
+
+ if (offset != null) {
+ boolean shouldTryToSimplify = RexUtil.isLiteral(fetch, false)
+ && RexUtil.isLiteral(offset, false);
+
+ fetch = builder.makeCall(IgniteSqlOperatorTable.PLUS, fetch,
offset);
+
+ if (shouldTryToSimplify && executor != null) {
+ try {
+ List<RexNode> result = new ArrayList<>();
+ executor.reduce(builder, List.of(fetch), result);
+
+ assert result.size() <= 1 : result;
+
+ if (result.size() == 1) {
+ fetch = result.get(0);
+ }
+ } catch (Exception ex) {
+ if (IgniteUtils.assertionsEnabled()) {
Review Comment:
LogicalRelImplementor expects fetch/offset to be literals / or dynamic
parameters.
So the assignment
> fetch = builder.makeCall(IgniteSqlOperatorTable.PLUS, fetch, offset);
is going to produce a physical operator tree that can not be converted to an
execution tree.
so If think the code should be something like
> // If we can optimise then
> RexCall tmp = builder.makeCall(IgniteSqlOperatorTable.PLUS, fetch,
offset);
> // ... if tmp was reduced to a constant, then return tmp
> // otherwise return fetch
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
