Review and tidy solr/modules/sql code

solr/modules/sql/src/java/org/apache/solr/handler/sql/CalciteSolrDriver.java

@@ -75,8 +75,8 @@ public Connection connect(String url, Properties info) throws SQLException {
     Hook.SQL2REL_CONVERTER_CONFIG_BUILDER.addThread(CalciteSolrDriver::subQueryThreshold);
 
     // disable Calcite's simplify (see SOLR-16009) as it erases some query
-    // constructs that are still meaningful to Solr (such as AND'd filters on the same field,
-    // which works for multi-valued fields in Solr but looks like nonsense to Calcite.
+    // constructs that are still meaningful to Solr (such as AND'd filters on the same field),
+    // which works for multivalued fields in Solr but looks like nonsense to Calcite.
     Hook.REL_BUILDER_SIMPLIFY.addThread(CalciteSolrDriver::relBuilderSimplify);
 
     Connection connection = super.connect(url, info);

solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrAggregate.java

@@ -48,7 +48,7 @@ class SolrAggregate extends Aggregate implements SolrRel {
 
   // Returns the Solr agg metric identifier (includes column) for the SQL metric
   static String solrAggMetricId(String metric, String column) {
-    // CountDistinctMetric's getIdentifer returns "countDist" but all others return a lowercased
+    // CountDistinctMetric's getIdentifier returns "countDist" but all others return a lowercased
     // value
     String funcName =
         COUNT_DISTINCT.equals(metric) ? COUNT_DISTINCT : metric.toLowerCase(Locale.ROOT);
@@ -118,7 +118,7 @@ private Pair<String, String> toSolrMetric(
           return new Pair<>(aggregation.getName(), "*");
         }
       case 1:
-        String inName = inNames.get(args.get(0));
+        String inName = inNames.get(args.getFirst());
         String name = implementor.fieldMappings.getOrDefault(inName, inName);
         if (SUPPORTED_AGGREGATIONS.contains(aggregation)) {
           return new Pair<>(aggregation.getName(), name);

solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrEnumerator.java

@@ -62,7 +62,7 @@ class SolrEnumerator implements Enumerator<Object> {
   @Override
   public Object current() {
     if (fields.size() == 1) {
-      return this.getter(current, fields.get(0));
+      return this.getter(current, fields.getFirst());
     } else {
       // Build an array with all fields in this row
       Object[] row = new Object[fields.size()];

solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrFilter.java

@@ -271,7 +271,7 @@ protected String translateIsNullOrIsNotNull(RexNode node) {
         throw new AssertionError("expected 1 operand for " + node);
       }
 
-      final RexNode left = operands.get(0);
+      final RexNode left = operands.getFirst();
       if (left instanceof RexInputRef) {
         String name = fieldNames.get(((RexInputRef) left).getIndex());
         SqlKind kind = node.getKind();
@@ -325,23 +325,23 @@ protected String translateAnd(RexNode node0) {
           }
         }
 
-        String query = "";
+        StringBuilder query = new StringBuilder();
         if (!andStrings.isEmpty()) {
           String andString = String.join(" AND ", andStrings);
-          query += "(" + andString + ")";
+          query.append("(").append(andString).append(")");
         }
         if (!notStrings.isEmpty()) {
           if (!query.isEmpty()) {
-            query += " AND ";
+            query.append(" AND ");
           }
           for (int i = 0; i < notStrings.size(); i++) {
             if (i > 0) {
-              query += " AND ";
+              query.append(" AND ");
             }
-            query += " (*:* -" + notStrings.get(i) + ")";
+            query.append(" (*:* -").append(notStrings.get(i)).append(")");
           }
         }
-        return query.trim();
+        return query.toString().trim();
       } else {
         return String.join(" AND ", andStrings);
       }
@@ -402,7 +402,7 @@ private String translateLikeTermToSolrSyntax(String term, Character escapeChar)
     protected String translateComparison(RexNode node) {
       final SqlKind kind = node.getKind();
       if (kind == SqlKind.NOT) {
-        RexNode negated = ((RexCall) node).getOperands().get(0);
+        RexNode negated = ((RexCall) node).getOperands().getFirst();
         if (negated.isA(SqlKind.AND)) {
           AndClause andClause = translateAndOrBetween(negated, true);
           // if the resulting andClause is a "between" then don't negate it as it's already
@@ -789,7 +789,7 @@ protected String translateAnd(RexNode node0) {
           notBuilder.append(")");
         }
 
-        return "and(" + builder.toString() + "," + notBuilder.toString() + ")";
+        return "and(" + builder + "," + notBuilder + ")";
       } else {
         return builder.toString();
       }

solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrSchema.java

@@ -84,10 +84,6 @@ public void close() {
     isClosed = true;
   }
 
-  public boolean isClosed() {
-    return isClosed;
-  }
-
   @Override
   protected Map<String, Table> getTableMap() {
     CloudSolrClient cloudSolrClient = solrClientCache.getCloudSolrClient(solrConnection);
@@ -160,8 +156,8 @@ RelDataType getRowSchema(String collection) {
 
   RelDataType buildRowSchema(String collection) {
     // Temporary type factory, just for the duration of this method. Allowable
-    // because we're creating a proto-type, not a type; before being used, the
-    // proto-type will be copied into a real type factory.
+    // because we're creating a prototype, not a type; before being used, the
+    // prototype will be copied into a real type factory.
     final RelDataTypeFactory typeFactory = new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
     final RelDataTypeFactory.Builder fieldInfo = typeFactory.builder();
 
@@ -201,12 +197,12 @@ RelDataType buildRowSchema(String collection) {
 
       RelDataType type;
 
-      // We have to pass multi-valued fields through Calcite as SQL Type ANY
+      // We have to pass multivalued fields through Calcite as SQL Type ANY
       // Array doesn't work for aggregations! Calcite doesn't like GROUP BY on an ARRAY field
-      // but Solr happily computes aggs on a multi-valued field, so we have a paradigm mis-match and
-      // ANY is the best way to retain use of operators on multi-valued fields while still being
+      // but Solr happily computes aggs on a multivalued field, so we have a paradigm mismatch and
+      // ANY is the best way to retain use of operators on multivalued fields while still being
       // able
-      // to GROUP BY and project the multi-valued fields in results
+      // to GROUP BY and project the multivalued fields in results
       EnumSet<FieldFlag> flags = getFieldFlags(luceneFieldInfo);
       if (flags != null && flags.contains(FieldFlag.MULTI_VALUED)) {
         type = typeFactory.createSqlType(SqlTypeName.ANY);

solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrTable.java

@@ -29,7 +29,6 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.apache.calcite.adapter.java.AbstractQueryableTable;
 import org.apache.calcite.linq4j.AbstractEnumerable;
 import org.apache.calcite.linq4j.Enumerable;
@@ -143,7 +142,7 @@ private Enumerable<Object> query(
     boolean mapReduce = "map_reduce".equals(properties.getProperty("aggregationMode"));
     boolean negative = Boolean.parseBoolean(negativeQuery);
 
-    String q = null;
+    String q;
 
     if (query == null) {
       q = DEFAULT_QUERY;
@@ -212,22 +211,6 @@ public Enumerator<Object> enumerator() {
     };
   }
 
-  private static StreamComparator bucketSortComp(List<Bucket> buckets, Map<String, String> dirs) {
-    FieldComparator[] comps = new FieldComparator[buckets.size()];
-    for (int i = 0; i < buckets.size(); i++) {
-      ComparatorOrder comparatorOrder =
-          ComparatorOrder.fromString(dirs.get(buckets.get(i).toString()));
-      String sortKey = buckets.get(i).toString();
-      comps[i] = new FieldComparator(sortKey, comparatorOrder);
-    }
-
-    if (comps.length == 1) {
-      return comps[0];
-    } else {
-      return new MultipleFieldComparator(comps);
-    }
-  }
-
   private static StreamComparator bucketSortComp(Bucket[] buckets, String dir) {
     FieldComparator[] comps = new FieldComparator[buckets.length];
     for (int i = 0; i < buckets.length; i++) {
@@ -267,8 +250,8 @@ private StreamComparator getComp(List<? extends Map.Entry<String, String>> order
 
   private List<Metric> buildMetrics(List<Pair<String, String>> metricPairs, boolean ifEmptyCount) {
     List<Metric> metrics = new ArrayList<>(metricPairs.size());
-    metrics.addAll(metricPairs.stream().map(this::getMetric).collect(Collectors.toList()));
-    if (metrics.size() == 0 && ifEmptyCount) {
+    metrics.addAll(metricPairs.stream().map(this::getMetric).toList());
+    if (metrics.isEmpty() && ifEmptyCount) {
       metrics.add(new CountMetric());
     }
     return metrics;
@@ -376,7 +359,7 @@ private TupleStream handleSelect(
   private String getSort(List<Pair<String, String>> orders) {
     StringBuilder buf = new StringBuilder();
     for (Pair<String, String> pair : orders) {
-      if (buf.length() > 0) {
+      if (!buf.isEmpty()) {
         buf.append(",");
       }
       buf.append(pair.getKey()).append(" ").append(pair.getValue());
@@ -385,17 +368,11 @@ private String getSort(List<Pair<String, String>> orders) {
     return buf.toString();
   }
 
-  private String getSingleSort(Pair<String, String> order) {
-    StringBuilder buf = new StringBuilder();
-    buf.append(order.getKey()).append(" ").append(order.getValue());
-    return buf.toString();
-  }
-
   private String getFields(List<Map.Entry<String, Class<?>>> fields) {
     StringBuilder buf = new StringBuilder();
     for (Map.Entry<String, Class<?>> field : fields) {
 
-      if (buf.length() > 0) {
+      if (!buf.isEmpty()) {
         buf.append(",");
       }
 
@@ -409,7 +386,7 @@ private String getFields(Set<String> fieldSet) {
     StringBuilder buf = new StringBuilder();
     for (String field : fieldSet) {
 
-      if (buf.length() > 0) {
+      if (!buf.isEmpty()) {
         buf.append(",");
       }
 
@@ -437,7 +414,7 @@ private Set<String> getFieldSet(Metric[] metrics, List<Map.Entry<String, Class<?
   }
 
   private static String getSortDirection(List<Pair<String, String>> orders) {
-    if (orders != null && orders.size() > 0) {
+    if (orders != null && !orders.isEmpty()) {
       for (Pair<String, String> item : orders) {
         return item.getValue();
       }
@@ -533,7 +510,7 @@ private TupleStream handleGroupByMapReduce(
     Set<String> fieldSet = getFieldSet(metrics, fields);
 
     if (metrics.length == 0) {
-      throw new IOException("Group by queries must include atleast one aggregate function.");
+      throw new IOException("Group by queries must include at least one aggregate function.");
     }
 
     String fl = getFields(fieldSet);
@@ -552,7 +529,7 @@ private TupleStream handleGroupByMapReduce(
 
     params.set(SORT, sort);
 
-    TupleStream tupleStream = null;
+    TupleStream tupleStream;
 
     // Always use the /export handler for Group By Queries because it requires exporting full
     // result sets.
@@ -603,7 +580,7 @@ private TupleStream handleGroupByMapReduce(
     //      We need to push down the having clause to ensure that LIMIT does not cut off records
     // prior to the having filter.
 
-    if (orders != null && orders.size() > 0) {
+    if (orders != null && !orders.isEmpty()) {
       if (!sortsEqual(buckets, sortDirection, orders)) {
         int lim = (limit == null) ? 100 : Integer.parseInt(limit);
         StreamComparator comp = getComp(orders);
@@ -678,9 +655,9 @@ private TupleStream handleGroupByFacet(
 
     int limit = lim != null ? Integer.parseInt(lim) : 1000;
 
-    FieldComparator[] sorts = null;
+    FieldComparator[] sorts;
 
-    if (orders == null || orders.size() == 0) {
+    if (orders == null || orders.isEmpty()) {
       sorts = new FieldComparator[buckets.length];
       for (int i = 0; i < sorts.length; i++) {
         sorts[i] = new FieldComparator("index", ComparatorOrder.ASCENDING);
@@ -743,11 +720,11 @@ private TupleStream handleSelectDistinctMapReduce(
 
     String fl = getFields(fields);
 
-    String sort = null;
-    StreamEqualitor ecomp = null;
-    StreamComparator comp = null;
+    String sort;
+    StreamEqualitor ecomp;
+    StreamComparator comp;
 
-    if (orders != null && orders.size() > 0) {
+    if (orders != null && !orders.isEmpty()) {
       StreamComparator[] adjustedSorts = adjustSorts(orders, buckets);
       // Because of the way adjustSorts works we know that each FieldComparator has a single
       // field name. For this reason we can just look at the leftFieldName
@@ -810,7 +787,7 @@ private TupleStream handleSelectDistinctMapReduce(
 
     params.set(SORT, sort);
 
-    TupleStream tupleStream = null;
+    TupleStream tupleStream;
 
     // Always use the /export handler for Distinct Queries because it requires exporting full
     // result sets.

solr/modules/sql/src/java/org/apache/solr/handler/sql/functions/ArrayContainsAll.java

@@ -18,9 +18,9 @@
 package org.apache.solr.handler.sql.functions;
 
 /**
- * Operator for filtering on Solr multi-valued fields with 'AND" clause. Example:
+ * Operator for filtering on Solr multivalued fields with "AND" clause. Example:
  * ARRAY_CONTAINS_ALL(field, ('val1', 'val2')) will be transformed to filter query field:("val1" AND
- * "val2")
+ * "val2").
  */
 public class ArrayContainsAll extends ArrayContains {
   private static final String UDF_NAME = "ARRAY_CONTAINS_ALL";

solr/modules/sql/src/java/org/apache/solr/handler/sql/functions/ArrayContainsAny.java

@@ -18,9 +18,9 @@
 package org.apache.solr.handler.sql.functions;
 
 /**
- * Operator for filtering on Solr multi-valued fields with 'OR" clause. Example:
+ * Operator for filtering on Solr multivalued fields with "OR" clause. Example:
  * ARRAY_CONTAINS_ALL(field, ('val1', 'val2')) will be transformed to filter query field:("val1" OR
- * "val2")
+ * "val2").
  */
 public class ArrayContainsAny extends ArrayContains {
   private static final String UDF_NAME = "ARRAY_CONTAINS_ANY";

solr/modules/sql/src/test/org/apache/solr/handler/sql/SQLWithAuthzEnabledTest.java

@@ -108,7 +108,7 @@ public void testSqlAuthz() throws Exception {
 
     ModifiableSolrParams params = new ModifiableSolrParams();
     params.set("stmt", "select id from " + collectionName);
-    String baseUrl = cluster.getJettySolrRunners().get(0).getBaseUrl().toString();
+    String baseUrl = cluster.getJettySolrRunners().getFirst().getBaseUrl().toString();
     SolrStream solrStream = new SolrStream(baseUrl, collectionName, "/sql", params);
     solrStream.setCredentials(SAD_USER, PASS);