Skip to content

Commit 57dfe15

Browse files
fang-xing-esqlfang-xing-esql
committed
add trace and clean up
1 parent a76362f commit 57dfe15

File tree

5 files changed

+127
-24
lines changed

5 files changed

+127
-24
lines changed

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Bucket.java‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import org.elasticsearch.common.io.stream.StreamOutput;
1515
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
1616
import org.elasticsearch.core.TimeValue;
17+
import org.elasticsearch.logging.LogManager;
18+
import org.elasticsearch.logging.Logger;
1719
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1820
import org.elasticsearch.xpack.esql.capabilities.PostOptimizationVerificationAware;
1921
import org.elasticsearch.xpack.esql.common.Failures;
@@ -59,6 +61,7 @@
5961
import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime;
6062
import static org.elasticsearch.xpack.esql.expression.Validations.isFoldable;
6163
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToLong;
64+
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateWithTypeToString;
6265

6366
/**
6467
* Splits dates and numbers into a given number of buckets. There are two ways to invoke
@@ -73,6 +76,8 @@ public class Bucket extends GroupingFunction.EvaluatableGroupingFunction
7376
SurrogateExpression {
7477
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Bucket", Bucket::new);
7578

79+
private static final Logger logger = LogManager.getLogger(Bucket.class);
80+
7681
// TODO maybe we should just cover the whole of representable dates here - like ten years, 100 years, 1000 years, all the way up.
7782
// That way you never end up with more than the target number of buckets.
7883
private static final Rounding LARGEST_HUMAN_DATE_ROUNDING = Rounding.builder(Rounding.DateTimeUnit.YEAR_OF_CENTURY).build();
@@ -519,15 +524,19 @@ public Expression surrogate(SearchStats searchStats) {
519524
var min = searchStats.min(fieldName);
520525
var max = searchStats.max(fieldName);
521526
// If min/max is available create rounding with them
522-
if (min != null && max != null && buckets().foldable()) {
523-
// System.out.println("field: " + fieldName + ", min: " + min + ", " + dateWithTypeToString((Long) min, fieldType));
524-
// System.out.println("field: " + fieldName + ", max: " + max + ", " + dateWithTypeToString((Long) max, fieldType));
525-
Rounding.Prepared rounding = getDateRounding(FoldContext.small(), (Long) min, (Long) max);
527+
if (min instanceof Long minValue && max instanceof Long maxValue && buckets().foldable()) {
528+
Rounding.Prepared rounding = getDateRounding(FoldContext.small(), minValue, maxValue);
526529
long[] roundingPoints = rounding.fixedRoundingPoints();
527-
// the min/max long values for date and date_nanos are correct, however the roundingPoints for date_nanos is null
528-
// System.out.println("roundingPoints = " + Arrays.toString(roundingPoints));
529530
if (roundingPoints == null) {
530-
return null; // TODO log this case
531+
logger.trace(
532+
"Fixed rounding point is null for field {}, minValue {} in string format {} and maxValue {} in string format {}",
533+
fieldName,
534+
minValue,
535+
dateWithTypeToString(minValue, fieldType),
536+
maxValue,
537+
dateWithTypeToString(maxValue, fieldType)
538+
);
539+
return null;
531540
}
532541
// Convert to round_to function with the roundings
533542
List<Expression> points = Arrays.stream(roundingPoints)

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTrunc.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.elasticsearch.compute.ann.Fixed;
1717
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
1818
import org.elasticsearch.core.TimeValue;
19+
import org.elasticsearch.logging.LogManager;
20+
import org.elasticsearch.logging.Logger;
1921
import org.elasticsearch.xpack.esql.core.expression.Expression;
2022
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
2123
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
@@ -50,6 +52,7 @@
5052
import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME;
5153
import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS;
5254
import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime;
55+
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateWithTypeToString;
5356

5457
public class DateTrunc extends EsqlScalarFunction implements SurrogateExpression {
5558
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
@@ -58,6 +61,8 @@ public class DateTrunc extends EsqlScalarFunction implements SurrogateExpression
5861
DateTrunc::new
5962
);
6063

64+
private static final Logger logger = LogManager.getLogger(DateTrunc.class);
65+
6166
@FunctionalInterface
6267
public interface DateTruncFactoryProvider {
6368
ExpressionEvaluator.Factory apply(Source source, ExpressionEvaluator.Factory lhs, Rounding.Prepared rounding);
@@ -231,8 +236,8 @@ private static Rounding.Prepared createRounding(final Period period, final ZoneI
231236

232237
rounding.timeZone(timeZone);
233238
if (min != null && max != null && tryPrepareWithMinMax) {
234-
// Multiple quantities of month, quarter or year is not supported by PreparedRounding.maybeUseArray, which is called by
235-
// prepare(min, max), as it may hit an assert in it.
239+
// Multiple quantities calendar interval - day/week/month/quarter/year is not supported by PreparedRounding.maybeUseArray,
240+
// which is called by prepare(min, max), as it may hit an assert. Call prepare(min, max) only for single calendar interval.
236241
return rounding.build().prepare(min, max);
237242
}
238243
return rounding.build().prepareForUnknown();
@@ -295,17 +300,20 @@ public Expression surrogate(SearchStats searchStats) {
295300
var min = searchStats.min(fieldName);
296301
var max = searchStats.max(fieldName);
297302
// If min/max is available create rounding with them
298-
if (min != null && max != null && interval().foldable()) {
299-
// System.out.println("field: "+ fieldName + ", min string: " + dateWithTypeToString((Long) min, fieldType));
300-
// System.out.println("field: "+ fieldName + ", max string: " + dateWithTypeToString((Long) max, fieldType));
303+
if (min instanceof Long minValue && max instanceof Long maxValue && interval().foldable()) {
301304
Object foldedInterval = interval().fold(FoldContext.small() /* TODO remove me */);
302-
Rounding.Prepared rounding = createRounding(foldedInterval, DEFAULT_TZ, (Long) min, (Long) max);
305+
Rounding.Prepared rounding = createRounding(foldedInterval, DEFAULT_TZ, minValue, maxValue);
303306
long[] roundingPoints = rounding.fixedRoundingPoints();
304-
// the min/max long values for date and date_nanos are correct, however the roundingPoints for date_nanos is null
305-
// System.out.println("field name = " + fieldName + ", min = " + min + ", max = " + max + ", roundingPoints = " +
306-
// Arrays.toString(roundingPoints));
307307
if (roundingPoints == null) {
308-
return null; // TODO log this case
308+
logger.trace(
309+
"Fixed rounding point is null for field {}, minValue {} in string format {} and maxValue {} in string format {}",
310+
fieldName,
311+
minValue,
312+
dateWithTypeToString(minValue, fieldType),
313+
maxValue,
314+
dateWithTypeToString(maxValue, fieldType)
315+
);
316+
return null;
309317
}
310318
// Convert to round_to function with the roundings
311319
List<Expression> points = Arrays.stream(roundingPoints)

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ public Object min(FieldName field) {
209209
Holder<Boolean> foundMinValue = new Holder<>(false);
210210
doWithContexts(r -> {
211211
byte[] minPackedValue = PointValues.getMinPackedValue(r, field.string());
212-
if (minPackedValue != null) {
212+
if (minPackedValue != null && minPackedValue.length == 8) {
213213
long minValue = NumericUtils.sortableBytesToLong(minPackedValue, 0);
214214
if (minValue <= min[0]) {
215215
min[0] = minValue;
@@ -236,7 +236,7 @@ public Object max(FieldName field) {
236236
Holder<Boolean> foundMaxValue = new Holder<>(false);
237237
doWithContexts(r -> {
238238
byte[] maxPackedValue = PointValues.getMaxPackedValue(r, field.string());
239-
if (maxPackedValue != null) {
239+
if (maxPackedValue != null && maxPackedValue.length == 8) {
240240
long maxValue = NumericUtils.sortableBytesToLong(maxPackedValue, 0);
241241
if (maxValue >= max[0]) {
242242
max[0] = maxValue;

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.elasticsearch.xpack.esql.core.type.InvalidMappedField;
3232
import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
3333
import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Case;
34+
import org.elasticsearch.xpack.esql.expression.function.scalar.math.RoundTo;
3435
import org.elasticsearch.xpack.esql.expression.function.scalar.nulls.Coalesce;
3536
import org.elasticsearch.xpack.esql.expression.function.scalar.string.StartsWith;
3637
import org.elasticsearch.xpack.esql.expression.function.scalar.string.regex.RLike;
@@ -51,6 +52,7 @@
5152
import org.elasticsearch.xpack.esql.plan.logical.MvExpand;
5253
import org.elasticsearch.xpack.esql.plan.logical.Project;
5354
import org.elasticsearch.xpack.esql.plan.logical.Row;
55+
import org.elasticsearch.xpack.esql.plan.logical.TopN;
5456
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
5557
import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject;
5658
import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation;
@@ -83,6 +85,7 @@
8385
import static org.elasticsearch.xpack.esql.EsqlTestUtils.unboundLogicalOptimizerContext;
8486
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
8587
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
88+
import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME;
8689
import static org.hamcrest.Matchers.contains;
8790
import static org.hamcrest.Matchers.equalTo;
8891
import static org.hamcrest.Matchers.hasSize;
@@ -774,6 +777,89 @@ public void testGroupingByMissingFields() {
774777
as(eval.child(), EsRelation.class);
775778
}
776779

780+
public void testSubstituteDateTruncInEvalWithRoundTo() {
781+
var plan = plan("""
782+
from test
783+
| sort hire_date
784+
| eval x = date_trunc(1 day, hire_date)
785+
| keep emp_no, hire_date, x
786+
| limit 5
787+
""");
788+
789+
// create a SearchStats with min and max millis
790+
Map<String, Object> minValue = Map.of("hire_date", 1697804103360L); // 2023-10-20T12:15:03.360Z
791+
Map<String, Object> maxValue = Map.of("hire_date", 1698069301543L); // 2023-10-23T13:55:01.543Z
792+
SearchStats searchStats = new EsqlTestUtils.TestSearchStatsWithMinMax(minValue, maxValue);
793+
794+
LogicalPlan localPlan = localPlan(plan, searchStats);
795+
Project project = as(localPlan, Project.class);
796+
TopN topN = as(project.child(), TopN.class);
797+
Eval eval = as(topN.child(), Eval.class);
798+
List<Alias> fields = eval.fields();
799+
assertEquals(1, fields.size());
800+
Alias a = fields.get(0);
801+
assertEquals("x", a.name());
802+
RoundTo roundTo = as(a.child(), RoundTo.class);
803+
FieldAttribute fa = as(roundTo.field(), FieldAttribute.class);
804+
assertEquals("hire_date", fa.name());
805+
assertEquals(DATETIME, fa.dataType());
806+
assertEquals(4, roundTo.points().size()); // 4 days
807+
EsRelation relation = as(eval.child(), EsRelation.class);
808+
}
809+
810+
public void testSubstituteDateTruncInAggWithRoundTo() {
811+
var plan = plan("""
812+
from test
813+
| stats count(*) by x = date_trunc(1 day, hire_date)
814+
""");
815+
816+
// create a SearchStats with min and max millis
817+
Map<String, Object> minValue = Map.of("hire_date", 1697804103360L); // 2023-10-20T12:15:03.360Z
818+
Map<String, Object> maxValue = Map.of("hire_date", 1698069301543L); // 2023-10-23T13:55:01.543Z
819+
SearchStats searchStats = new EsqlTestUtils.TestSearchStatsWithMinMax(minValue, maxValue);
820+
821+
LogicalPlan localPlan = localPlan(plan, searchStats);
822+
Limit limit = as(localPlan, Limit.class);
823+
Aggregate aggregate = as(limit.child(), Aggregate.class);
824+
Eval eval = as(aggregate.child(), Eval.class);
825+
List<Alias> fields = eval.fields();
826+
assertEquals(1, fields.size());
827+
Alias a = fields.get(0);
828+
assertEquals("x", a.name());
829+
RoundTo roundTo = as(a.child(), RoundTo.class);
830+
FieldAttribute fa = as(roundTo.field(), FieldAttribute.class);
831+
assertEquals("hire_date", fa.name());
832+
assertEquals(DATETIME, fa.dataType());
833+
assertEquals(4, roundTo.points().size()); // 4 days
834+
EsRelation relation = as(eval.child(), EsRelation.class);
835+
}
836+
837+
public void testSubstituteBucketInAggWithRoundTo() {
838+
var plan = plan("""
839+
from test
840+
| stats count(*) by x = bucket(hire_date, 1 day)
841+
""");
842+
// create a SearchStats with min and max millis
843+
Map<String, Object> minValue = Map.of("hire_date", 1697804103360L); // 2023-10-20T12:15:03.360Z
844+
Map<String, Object> maxValue = Map.of("hire_date", 1698069301543L); // 2023-10-23T13:55:01.543Z
845+
SearchStats searchStats = new EsqlTestUtils.TestSearchStatsWithMinMax(minValue, maxValue);
846+
847+
LogicalPlan localPlan = localPlan(plan, searchStats);
848+
Limit limit = as(localPlan, Limit.class);
849+
Aggregate aggregate = as(limit.child(), Aggregate.class);
850+
Eval eval = as(aggregate.child(), Eval.class);
851+
List<Alias> fields = eval.fields();
852+
assertEquals(1, fields.size());
853+
Alias a = fields.get(0);
854+
assertEquals("x", a.name());
855+
RoundTo roundTo = as(a.child(), RoundTo.class);
856+
FieldAttribute fa = as(roundTo.field(), FieldAttribute.class);
857+
assertEquals("hire_date", fa.name());
858+
assertEquals(DATETIME, fa.dataType());
859+
assertEquals(4, roundTo.points().size()); // 4 days
860+
EsRelation relation = as(eval.child(), EsRelation.class);
861+
}
862+
777863
private IsNotNull isNotNull(Expression field) {
778864
return new IsNotNull(EMPTY, field);
779865
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/stats/SearchContextStatsTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ public void setup() throws IOException {
4545
List<SearchExecutionContext> contexts = new ArrayList<>(indexCount);
4646
mapperServices = new ArrayList<>(indexCount);
4747
readers = new ArrayList<>(indexCount);
48-
minMillis = dateTimeToLong("2025-01-01T00:00:01");
49-
minNanos = dateNanosToLong("2025-01-01T00:00:01");
50-
maxMillis = minMillis;
51-
maxNanos = minNanos;
48+
maxMillis = minMillis = dateTimeToLong("2025-01-01T00:00:01");
49+
maxNanos = minNanos = dateNanosToLong("2025-01-01T00:00:01");
50+
5251
MapperServiceTestCase mapperHelper = new MapperServiceTestCase() {
5352
};
53+
// create one or more index, so that there is one or more SearchExecutionContext in SearchStats
5454
for (int i = 0; i < indexCount; i++) {
55-
// Start with numeric, millis/nanos, keyword types in the index mapping, more data types can be covered later if necessary.
55+
// Start with millis/nanos, numeric and keyword types in the index mapping, more data types can be covered later if needed.
5656
// SearchContextStats returns min/max for millis and nanos only currently, null is returned for the other types min and max.
5757
MapperService mapperService;
5858
if (i == 0) {

0 commit comments

Comments
 (0)