kgyrtkirk commented on code in PR #16382:
URL: https://github.com/apache/druid/pull/16382#discussion_r1593839599


##########
sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java:
##########
@@ -36,77 +39,200 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.Function;
 
 /**
- * Annotation to specify desired framework settings.
+ * Specifies current framework settings.
  *
- * This class provides junit rule facilities to build the framework accordingly
- * to the annotation. These rules also cache the previously created frameworks.
+ * Intended usage from tests is via the annotations:
+ *   @SqlTestFrameworkConfig.MinTopNThreshold(33)
+ *
+ * In case of annotations used; it picks up all annotations from:
+ *  * the method
+ *  * its enclosing class and its parents
+ * if none contains a specific setting the default is being taken.
+ *
+ * All configurable setting should have:
+ *   * an annotation with `value` with the desired type
+ *   * the annotation itself should be annotated with itslef to set the 
default value
+ *   * a field should be added to the main config class
  */
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.METHOD})
-public @interface SqlTestFrameworkConfig
+public class SqlTestFrameworkConfig
 {
-  int numMergeBuffers() default 0;
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target({ElementType.METHOD, ElementType.TYPE})
+  @NumMergeBuffers(0)
+  public @interface NumMergeBuffers
+  {
+    int value();
+  }
 
-  int minTopNThreshold() default TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD;
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target({ElementType.METHOD, ElementType.TYPE})
+  @MinTopNThreshold(TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD)
+  public @interface MinTopNThreshold
+  {
+    int value();
+  }
 
-  ResultCacheMode resultCache() default ResultCacheMode.DISABLED;
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target({ElementType.METHOD, ElementType.TYPE})
+  @ResultCache(ResultCacheMode.DISABLED)
+  public @interface ResultCache
+  {
+    ResultCacheMode value();
+  }
 
   /**
-   * Non-annotation version of {@link SqlTestFrameworkConfig}.
-   *
-   * Makes it less convoluted to work with configurations created at runtime.
+   * Declares which {@link QueryComponentSupplier} must be used for the class.
    */
-  class SqlTestFrameworkConfigInstance
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target({ElementType.METHOD, ElementType.TYPE})
+  @Supplier(StandardComponentSupplier.class)
+  public @interface Supplier
   {
-    public final int numMergeBuffers;
-    public final int minTopNThreshold;
-    public final ResultCacheMode resultCache;
+    Class<? extends QueryComponentSupplier> value();
+  }
 
-    public SqlTestFrameworkConfigInstance(SqlTestFrameworkConfig annotation)
-    {
-      numMergeBuffers = annotation.numMergeBuffers();
-      minTopNThreshold = annotation.minTopNThreshold();
-      resultCache = annotation.resultCache();
+  public final int numMergeBuffers;
+  public final int minTopNThreshold;
+  public final ResultCacheMode resultCache;
+  public final Class<? extends QueryComponentSupplier> supplier;
+
+  public SqlTestFrameworkConfig(List<Annotation> annotations)
+  {
+    try {
+      numMergeBuffers = getValueFromAnnotation(annotations, 
NumMergeBuffers.class);
+      minTopNThreshold = getValueFromAnnotation(annotations, 
MinTopNThreshold.class);
+      resultCache = getValueFromAnnotation(annotations, ResultCache.class);
+      supplier = getValueFromAnnotation(annotations, Supplier.class);
+    }
+    catch (NoSuchMethodException | SecurityException | IllegalAccessException 
| IllegalArgumentException
+        | InvocationTargetException e) {
+      throw new RuntimeException(e);
     }
+  }
 
-    @Override
-    public int hashCode()
-    {
-      return Objects.hash(minTopNThreshold, numMergeBuffers, resultCache);
+  public SqlTestFrameworkConfig(Map<String, String> queryParams)
+  {
+    try {
+      numMergeBuffers = getValueFromMap(queryParams, NumMergeBuffers.class);
+      minTopNThreshold = getValueFromMap(queryParams, MinTopNThreshold.class);
+      resultCache = getValueFromMap(queryParams, ResultCache.class);
+      supplier = getValueFromMap(queryParams, Supplier.class);
     }
+    catch (NoSuchMethodException | SecurityException | IllegalAccessException 
| IllegalArgumentException
+        | InvocationTargetException e) {
+      throw new RuntimeException(e);
+    }
+  }
 
-    @Override
-    public boolean equals(Object obj)
-    {
-      if (obj == null || getClass() != obj.getClass()) {
-        return false;
+  @SuppressWarnings("unchecked")
+  private <T> T getValueFromMap(Map<String, String> map, Class<? extends 
Annotation> annotationClass)
+      throws IllegalAccessException, InvocationTargetException, 
NoSuchMethodException, SecurityException
+  {
+    String value = map.get(annotationClass.getSimpleName());
+    if (value == null) {
+      return defaultValue(annotationClass);
+    }
+    Class<?> type = annotationClass.getMethod("value").getReturnType();
+
+    if (type == int.class) {
+      return (T) Integer.valueOf(value);
+    }
+    if (type == Class.class) {
+
+      Object clazz = getQueryComponentSupplierForName(value);
+      if (clazz != null) {
+        return (T) clazz;
+      }
+
+    }
+    throw new RuntimeException("don't know how to handle conversion to " + 
type);
+  }
+
+  private Object getQueryComponentSupplierForName(String name)
+  {
+    Set<Class<? extends QueryComponentSupplier>> subTypes = new 
Reflections("org.apache.druid")

Review Comment:
   this newly added by-name lookup will not interfere with existing tests; as 
in those cases a ComponentSupplier class is  specified as a java class 
explicitly in the annotation - however in case it comes from a jdbc-uri it will 
need the lookup.
   
   run all `Calcite*Test` on master and on this branch - didn't seen any 
relevant difference in the runtimes
   <details>
   <summary>testrun on master</summary>
   
   ```
   [INFO] -------------------------------------------------------
   [INFO]  T E S T S
   [INFO] -------------------------------------------------------
   [INFO] Running org.apache.druid.sql.calcite.CalciteNestedDataQueryTest
   [INFO] Tests run: 145, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
9.255 s -- in org.apache.druid.sql.calcite.CalciteNestedDataQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteArraysQueryTest
   [INFO] Tests run: 156, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
11.42 s -- in org.apache.druid.sql.calcite.CalciteArraysQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteUnionQueryTest
   [INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.881 s -- in org.apache.druid.sql.calcite.CalciteUnionQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSysQueryTest
   [WARN] Tests run: 4, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 2.646 
s -- in org.apache.druid.sql.calcite.CalciteSysQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest
   [INFO] Tests run: 90, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
7.361 s -- in org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteStrictInsertTest
   [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.488 
s -- in org.apache.druid.sql.calcite.CalciteStrictInsertTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSimpleQueryTest
   [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.734 s -- in org.apache.druid.sql.calcite.CalciteSimpleQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteParameterQueryTest
   [INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.874 s -- in org.apache.druid.sql.calcite.CalciteParameterQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteJoinQueryTest
   [WARN] Tests run: 586, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 
16.89 s -- in org.apache.druid.sql.calcite.CalciteJoinQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogInsertTest
   [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.637 s -- in org.apache.druid.sql.calcite.CalciteCatalogInsertTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteExplainQueryTest
   [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.514 
s -- in org.apache.druid.sql.calcite.CalciteExplainQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteExportTest
   [WARN] Tests run: 13, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 
2.472 s -- in org.apache.druid.sql.calcite.CalciteExportTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteWindowQueryTest
   [WARN] Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 
4.641 s -- in org.apache.druid.sql.calcite.CalciteWindowQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteTableAppendTest
   [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.469 s -- in org.apache.druid.sql.calcite.CalciteTableAppendTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteReplaceDmlTest
   [INFO] Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.769 s -- in org.apache.druid.sql.calcite.CalciteReplaceDmlTest
   [INFO] Running org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest
   [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.435 
s -- in org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest
   [INFO] Running org.apache.druid.sql.calcite.planner.CalcitesTest
   [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
s -- in org.apache.druid.sql.calcite.planner.CalcitesTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest
   [INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.919 s -- in org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteQueryTest
   [WARN] Tests run: 389, Failures: 0, Errors: 0, Skipped: 4, Time elapsed: 
13.97 s -- in org.apache.druid.sql.calcite.CalciteQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSubqueryTest
   [INFO] Tests run: 44, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.449 s -- in org.apache.druid.sql.calcite.CalciteSubqueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteInsertDmlTest
   [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.959 s -- in org.apache.druid.sql.calcite.CalciteInsertDmlTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSelectQueryTest
   [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.949 s -- in org.apache.druid.sql.calcite.CalciteSelectQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest
   [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.020 s -- in org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogReplaceTest
   [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.566 s -- in org.apache.druid.sql.calcite.CalciteCatalogReplaceTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest
   [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.472 
s -- in org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteScanSignatureTest
   [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.472 
s -- in org.apache.druid.sql.calcite.CalciteScanSignatureTest
   [INFO] 
   [INFO] Results:
   [INFO] 
   [WARN] Tests run: 1845, Failures: 0, Errors: 0, Skipped: 11
   [INFO] 
   [INFO] 
   [INFO] --- animal-sniffer-maven-plugin:1.23:check (check-java-api) @ 
druid-sql ---
   [INFO] Checking unresolved references to 
org.codehaus.mojo.signature:java18:1.0
   [INFO] Segment walltime 124 s, segment projects service time 124 s, 
effective/maximum degree of concurrency 1.00/11
   [INFO] 
------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] 
------------------------------------------------------------------------
   [INFO] Total time:  02:04 min (Wall Clock)
   [INFO] Finished at: 2024-05-08T10:54:19Z
   [INFO] 
------------------------------------------------------------------------
   ```
   
   </details>
   
   <details>
   <summary>testrun on branch</summary>
   
   ```
   [INFO] -------------------------------------------------------
   [INFO]  T E S T S
   [INFO] -------------------------------------------------------
   [INFO] Running org.apache.druid.sql.calcite.CalciteNestedDataQueryTest
   [INFO] Tests run: 145, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
9.195 s -- in org.apache.druid.sql.calcite.CalciteNestedDataQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteArraysQueryTest
   [INFO] Tests run: 156, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
11.17 s -- in org.apache.druid.sql.calcite.CalciteArraysQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteUnionQueryTest
   [INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.815 s -- in org.apache.druid.sql.calcite.CalciteUnionQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSysQueryTest
   [WARN] Tests run: 4, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 2.628 
s -- in org.apache.druid.sql.calcite.CalciteSysQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest
   [INFO] Tests run: 90, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
7.194 s -- in org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteStrictInsertTest
   [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.519 
s -- in org.apache.druid.sql.calcite.CalciteStrictInsertTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSimpleQueryTest
   [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.774 s -- in org.apache.druid.sql.calcite.CalciteSimpleQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteParameterQueryTest
   [INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.923 s -- in org.apache.druid.sql.calcite.CalciteParameterQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteJoinQueryTest
   [WARN] Tests run: 586, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 
16.70 s -- in org.apache.druid.sql.calcite.CalciteJoinQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogInsertTest
   [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.681 s -- in org.apache.druid.sql.calcite.CalciteCatalogInsertTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteExplainQueryTest
   [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.536 
s -- in org.apache.druid.sql.calcite.CalciteExplainQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteExportTest
   [WARN] Tests run: 13, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 
2.483 s -- in org.apache.druid.sql.calcite.CalciteExportTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteWindowQueryTest
   [WARN] Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 
4.509 s -- in org.apache.druid.sql.calcite.CalciteWindowQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteTableAppendTest
   [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.490 s -- in org.apache.druid.sql.calcite.CalciteTableAppendTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteReplaceDmlTest
   [INFO] Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.813 s -- in org.apache.druid.sql.calcite.CalciteReplaceDmlTest
   [INFO] Running org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest
   [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.428 
s -- in org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest
   [INFO] Running org.apache.druid.sql.calcite.planner.CalcitesTest
   [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 
s -- in org.apache.druid.sql.calcite.planner.CalcitesTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest
   [INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
4.061 s -- in org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteQueryTest
   [WARN] Tests run: 389, Failures: 0, Errors: 0, Skipped: 4, Time elapsed: 
14.12 s -- in org.apache.druid.sql.calcite.CalciteQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSubqueryTest
   [INFO] Tests run: 44, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.475 s -- in org.apache.druid.sql.calcite.CalciteSubqueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteInsertDmlTest
   [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.018 s -- in org.apache.druid.sql.calcite.CalciteInsertDmlTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteSelectQueryTest
   [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.973 s -- in org.apache.druid.sql.calcite.CalciteSelectQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest
   [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.062 s -- in org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogReplaceTest
   [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.681 s -- in org.apache.druid.sql.calcite.CalciteCatalogReplaceTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest
   [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.577 
s -- in org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest
   [INFO] Running org.apache.druid.sql.calcite.CalciteScanSignatureTest
   [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.448 
s -- in org.apache.druid.sql.calcite.CalciteScanSignatureTest
   [INFO] 
   [INFO] Results:
   [INFO] 
   [WARN] Tests run: 1845, Failures: 0, Errors: 0, Skipped: 11
   [INFO] 
   [INFO] 
   [INFO] --- animal-sniffer-maven-plugin:1.23:check (check-java-api) @ 
druid-sql ---
   [INFO] Checking unresolved references to 
org.codehaus.mojo.signature:java18:1.0
   [INFO] Segment walltime 126 s, segment projects service time 126 s, 
effective/maximum degree of concurrency 1.00/11
   [INFO] 
------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] 
------------------------------------------------------------------------
   [INFO] Total time:  02:06 min (Wall Clock)
   [INFO] Finished at: 2024-05-08T10:52:09Z
   [INFO] 
------------------------------------------------------------------------
   ```
   </details>
   
   



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to