asolimando commented on a change in pull request #2568:
URL: https://github.com/apache/calcite/pull/2568#discussion_r723934146
##########
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraRules.java
##########
@@ -222,19 +224,25 @@ RelNode convert(LogicalFilter filter, CassandraTableScan
scan) {
scan.cassandraTable.getClusteringOrder());
}
+ /** Deprecated in favor of CassandraFilterRuleConfig. **/
Review comment:
What about using `{@link CassandraFilterRuleConfig}`?
##########
File path: core/src/main/java/org/apache/calcite/plan/RelRule.java
##########
@@ -55,7 +55,14 @@
*
* <p>2. If your rule is not a sub-class of
* {@link org.apache.calcite.rel.convert.ConverterRule},
- * create an inner {@code interface Config extends RelRule.Config}.
+ * create an inner {@code interface Config extends RelRule.Config} and
+ * annotate it with {@code @Value.Immutable}. Note, if your inner class
+ * is two levels deep (e.g. top-level Rule with Config inside), we recommend
+ * you annotation the outer class with {@code @Value.Enclosing} which will
Review comment:
we recommend you to annotate?
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
##########
@@ -308,11 +308,15 @@ public JoinConditionPushRule(RelFactories.FilterFactory
filterFactory,
perform(call, null, join);
}
+ /** Deprecated, use JoinConditionPushRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ProjectToWindowRule.java
##########
@@ -179,16 +185,20 @@ public ProjectToLogicalProjectAndWindowRule(
call.transformTo(newRel);
}
+ /** Deprecated, use ProjectToLogicalProjectAndWindowRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraRules.java
##########
@@ -408,15 +420,19 @@ public RelNode convert(EnumerableLimit limit) {
}
}
+ /** Deprecated in favor of CassandraLimitRuleConfig. **/
Review comment:
@link here too in case the suggestion is adopted
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -214,11 +223,14 @@ protected DeltaSortTransposeRule(Config config) {
call.transformTo(newSort);
}
+ /** Deprecated, use DeltaSortTransposeRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -342,14 +360,18 @@ protected DeltaTableScanToEmptyRule(Config config) {
}
}
+ /** Deprecated, use DeltaTableScanToEmptyRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
##########
@@ -2539,14 +2546,17 @@ static Config config(RelDecorrelator d,
d.removeCorVarFromTree(correlate);
}
+ /** Deprecated, use RemoveCorrelationForScalarAggregateRuleConfig instead.
**/
Review comment:
@link here too
##########
File path:
innodb/src/main/java/org/apache/calcite/adapter/innodb/InnodbRules.java
##########
@@ -201,14 +203,19 @@ RelNode convert(LogicalFilter filter, InnodbTableScan
scan) {
return filter;
}
+ /** Deprecated, use InnodbFilterRuleConfig instead. **/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -780,14 +807,19 @@ private static RexNode stripFilter(RexNode node) {
return refs;
}
+ /** Deprecated, use DruidAggregateProjectRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraRules.java
##########
@@ -354,9 +362,14 @@ private static boolean collationsCompatible(RelCollation
sortCollation,
}
}
+ /** Deprecated in favor of CassandraSortRuleConfig. **/
Review comment:
Along the same line, maybe using `{@link CassandraSortRuleConfig}`?
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
##########
@@ -367,15 +371,19 @@ public FilterIntoJoinRule(boolean smart,
perform(call, filter, join);
}
+ /** Deprecated, use FilterIntoJoinRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/DateRangeRules.java
##########
@@ -215,11 +215,14 @@ assert requireNonNull(finder,
"finder").timeUnits.isEmpty() && finder.opKinds.is
call.transformTo(relBuilder.build());
}
+ /** Deprecated, use FilterDateRangeRuleConfig instead. **/
Review comment:
@link here too if OK
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -399,15 +402,18 @@ public JoinReduceExpressionsRule(Class<? extends Join>
joinClass,
call.getPlanner().prune(join);
}
+ /** Deprecated, use JoinReduceExpressionsRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ProjectToWindowRule.java
##########
@@ -103,11 +103,14 @@ protected CalcToWindowRule(Config config) {
call.transformTo(newRel);
}
+ /** Deprecated, use CalcToWindowRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -528,15 +534,18 @@ protected RelNode
createEmptyRelOrEquivalent(RelOptRuleCall call, Calc input) {
return call.builder().push(input).empty().build();
}
+ /** Deprecated, use CalcReduceExpressionsRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -172,11 +178,14 @@ protected DeltaAggregateTransposeRule(Config config) {
call.transformTo(newAggregate);
}
+ /** Deprecated, use DeltaAggregateTransposeRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -255,15 +255,18 @@ private void reduceNotNullableFilter(
}
}
+ /** Deprecated, use WindowReduceExpressionsRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
##########
@@ -344,21 +344,24 @@ private static boolean isEmpty(RelNode node) {
call.transformTo(emptyValues);
}
+ /** Deprecated, use RemoveEmptySingleRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/SemiJoinRule.java
##########
@@ -156,11 +156,14 @@ public ProjectToSemiJoinRule(Class<Project> projectClass,
perform(call, project, join, left, aggregate);
}
+ /** Deprecated, use ProjectToSemiJoinRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -130,11 +133,14 @@ protected DeltaFilterTransposeRule(Config config) {
call.transformTo(newFilter);
}
+ /** Deprecated, use DeltaFilterTransposeRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
##########
@@ -2154,14 +2158,16 @@ static Config config(RelDecorrelator decorrelator,
d.removeCorVarFromTree(correlate);
}
+ /** Deprecated, use RemoveCorrelationForScalarProjectRuleConfig instead.
**/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/SemiJoinRule.java
##########
@@ -210,11 +213,14 @@ public JoinToSemiJoinRule(
perform(call, null, join, left, aggregate);
}
+ /** Deprecated, use JoinToSemiJoinRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -252,11 +264,14 @@ protected DeltaUnionTransposeRule(Config config) {
call.transformTo(newUnion);
}
+ /** Deprecated, use DeltaUnionTransposeRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -622,15 +631,18 @@ public WindowReduceExpressionsRule(Class<? extends
Window> windowClass,
}
}
+ /** Deprecated, use WindowReduceExpressionsRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -415,11 +437,14 @@ public DeltaJoinTransposeRule() {
call.transformTo(newNode);
}
+ /** Deprecated, use DeltaJoinTransposeRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
##########
@@ -1936,10 +1936,13 @@ static Config config(RelBuilderFactory f) {
call.transformTo(relBuilder.build());
}
+ /** Deprecated, use RemoveSingleAggregateRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -90,11 +90,14 @@ protected DeltaProjectTransposeRule(Config config) {
call.transformTo(newProject);
}
+ /** Deprecated, use DeltaProjectTransposeRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
core/src/test/java/org/apache/calcite/plan/volcano/CollationConversionTest.java
##########
@@ -175,6 +178,8 @@ protected SingleNodeRule(Config config) {
}
/** Rule configuration. */
+ @Value.Immutable
+ @Value.Style(init = "with*", typeImmutable =
"ImmutableLeafTraitRuleConfig")
Review comment:
What about creating an interface like you did for `CalciteImmutable` to
apply the value consistently?
##########
File path: core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
##########
@@ -2707,22 +2717,24 @@ private void onMatch2(
call.transformTo(newOutput);
}
+ /** Deprecated, use RemoveCorrelationForScalarAggregateRuleConfig instead.
**/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -484,13 +500,19 @@ protected DruidPostAggregationProjectRule(Config config) {
}
}
+ /** Deprecated, use DruidPostAggregationProjectRuleConfig instead. **/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -829,13 +861,18 @@ protected DruidSortRule(Config config) {
call.transformTo(DruidQuery.extendQuery(query, newSort));
}
+ /** Deprecated, use DruidSortRuleConfig instead. **/
Review comment:
@link here too
##########
File path: core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java
##########
@@ -302,11 +317,14 @@ protected DeltaTableScanRule(Config config) {
}
}
+ /** Deprecated, use DeltaTableScanRuleConfig instead. **/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -279,13 +280,18 @@ protected DruidFilterRule(Config config) {
return ImmutableTriple.of(timeRangeNodes, pushableNodes,
nonPushableNodes);
}
+ /** Deprecated, use DruidFilterRuleConfig instead. **/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -538,13 +560,18 @@ protected DruidAggregateRule(Config config) {
call.transformTo(DruidQuery.extendQuery(query, newAggregate));
}
+ /** Deprecated, use DruidAggregateRuleConfig instead. **/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -422,13 +433,18 @@ protected DruidProjectRule(Config config) {
return Pair.of(aboveNodes, belowNodes);
}
+ /** Deprecated, use DruidProjectRuleConfig instead. **/
Review comment:
@link here too
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -324,13 +330,18 @@ protected DruidHavingFilterRule(Config config) {
}
}
+ /** Deprecated, use DruidHavingFilterRuleConfig instead. **/
Review comment:
link here too
##########
File path:
innodb/src/main/java/org/apache/calcite/adapter/innodb/InnodbRules.java
##########
@@ -310,9 +317,14 @@ protected InnodbSortFilterRule(Config config) {
return collationsCompatible(sort.getCollation(),
filter.getImplicitCollation());
}
+ /** Deprecated, use InnodbSortFilterRuleConfig instead. **/
Review comment:
@link here too
##########
File path:
innodb/src/main/java/org/apache/calcite/adapter/innodb/InnodbRules.java
##########
@@ -349,9 +361,14 @@ protected InnodbSortTableScanRule(Config config) {
return collationsCompatible(sort.getCollation(),
tableScan.getImplicitCollation());
}
+ /** Deprecated, use InnodbSortTableScanRuleConfig instead. **/
Review comment:
@link here too
--
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]
