mihaibudiu commented on code in PR #4606:
URL: https://github.com/apache/calcite/pull/4606#discussion_r2483811975


##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -6534,6 +6538,17 @@ public interface Config {
      * method, and use the default value of {@link #isExpand()}, false. */
     Config withExpand(boolean expand);
 
+    /** Returns the {@code optimizeOrderBy} option. Controls whether to 
optimize
+     * ORDER BY clauses using functional dependencies. If true (default is 
false),
+     * redundant ORDER BY items that are determined by earlier items will be
+     * removed based on metadata about functional dependencies. */
+    @Value.Default default boolean isOptimizeOrderBy() {

Review Comment:
   I also think that the name of the method is not specific enough. There may 
be many ways to optimize order by.
   You should call it maybe removeRedundantOrderbyColumns.



##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -6534,6 +6538,17 @@ public interface Config {
      * method, and use the default value of {@link #isExpand()}, false. */
     Config withExpand(boolean expand);
 
+    /** Returns the {@code optimizeOrderBy} option. Controls whether to 
optimize
+     * ORDER BY clauses using functional dependencies. If true (default is 
false),
+     * redundant ORDER BY items that are determined by earlier items will be
+     * removed based on metadata about functional dependencies. */
+    @Value.Default default boolean isOptimizeOrderBy() {

Review Comment:
   never fond of new configuration options; they double the space of tests one 
has to run.
   I hope the plan is to remove this after making it default to true.
   but this kind of stuff never happens.
   
   Normally the builder should not optimize stuff, it should just build what it 
is asked.
   I personally would strive to write a separate visitor which can perform this 
rewrite, even if it cannot be written as a standard optimization rule.
   But I think all this has been discussed in the Jira.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to