From 4841a880f179970280edc5ba9095ff0bdf72224d Mon Sep 17 00:00:00 2001 From: cecemei Date: Wed, 13 Aug 2025 23:54:16 -0700 Subject: [PATCH 01/18] some --- .../segment/DatasketchesProjectionTest.java | 2 +- .../common/granularity/Granularities.java | 56 +++- .../common/granularity/PeriodGranularity.java | 133 +++++++++ .../druid/math/expr/BinaryEvalOpExprBase.java | 7 + .../apache/druid/math/expr/ConstantExpr.java | 10 +- .../java/org/apache/druid/math/expr/Expr.java | 2 + .../druid/math/expr/ExprMacroTable.java | 25 +- .../druid/math/expr/FunctionalExpr.java | 14 +- .../druid/math/expr/IdentifierExpr.java | 8 + .../apache/druid/math/expr/LambdaExpr.java | 6 + .../druid/math/expr/UnaryOperatorExpr.java | 7 + .../expr/vector/FallbackVectorProcessor.java | 6 + .../expression/TimestampCeilExprMacro.java | 13 +- .../segment/projections/Projections.java | 13 +- .../virtual/ExpressionVirtualColumn.java | 2 +- .../granularity/QueryGranularityTest.java | 60 +++- .../util/common/PeriodGranularityTest.java | 136 +++++++++ .../segment/CursorFactoryProjectionTest.java | 278 +++++++++++++----- 18 files changed, 674 insertions(+), 104 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/segment/DatasketchesProjectionTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/segment/DatasketchesProjectionTest.java index 6073b0623032..51075d095f59 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/segment/DatasketchesProjectionTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/segment/DatasketchesProjectionTest.java @@ -227,7 +227,7 @@ private static IndexBuilder makeBuilder(DimensionsSpec dimensionsSpec, boolean a IncrementalIndexSchema.builder() .withDimensionsSpec(dimensionsSpec) .withRollup(false) - .withMinTimestamp(CursorFactoryProjectionTest.TIMESTAMP.getMillis()) + .withMinTimestamp(CursorFactoryProjectionTest.UTC_MIDNIGHT.getMillis()) .withProjections(autoSchema ? AUTO_PROJECTIONS : PROJECTIONS) .build() ) diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java index b3fa3e8ede72..243f5f955c49 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java @@ -37,7 +37,8 @@ import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; -import org.joda.time.chrono.ISOChronology; +import org.joda.time.DateTimeZone; +import org.joda.time.Duration; import javax.annotation.Nullable; import java.util.Arrays; @@ -163,10 +164,25 @@ public static ExpressionVirtualColumn toVirtualColumn(Granularity granularity, S expression = ColumnHolder.TIME_COLUMN_NAME; } else { PeriodGranularity period = (PeriodGranularity) granularity; - if (!ISOChronology.getInstanceUTC().getZone().equals(period.getTimeZone()) || period.getOrigin() != null) { + if (period.getOrigin() != null) { expression = ColumnHolder.TIME_COLUMN_NAME; - } else { + } else if (period.getTimeZone().equals(DateTimeZone.UTC)) { expression = TimestampFloorExprMacro.forQueryGranularity(period.getPeriod()); + } else if (period.getPeriod().getYears() != 0 || period.getPeriod().getMonths() != 0) { + if (PeriodGranularity.getStandardSeconds(period.getPeriod().withYears(0).withMonths(0)).isPresent()) { + expression = TimestampFloorExprMacro.forQueryGranularity(Duration.standardSeconds(1).toPeriod()); + } else { + // period has year & month, generally it should not have milliseconds, but this is a fallback + expression = ColumnHolder.TIME_COLUMN_NAME; + } + } else { + if (PeriodGranularity.getStandardSeconds(period.getPeriod()).isEmpty()) { + // period has milliseconds + expression = ColumnHolder.TIME_COLUMN_NAME; + } else { + int seconds = period.getUtcMappablePeriodSecondsOrThrow(); + expression = TimestampFloorExprMacro.forQueryGranularity(Duration.standardSeconds(seconds).toPeriod()); + } } } @@ -194,15 +210,35 @@ public static ExpressionVirtualColumn toVirtualColumn(Granularity granularity, S public static Granularity fromVirtualColumn(VirtualColumn virtualColumn) { if (virtualColumn instanceof ExpressionVirtualColumn) { - final ExpressionVirtualColumn expressionVirtualColumn = (ExpressionVirtualColumn) virtualColumn; - final Expr expr = expressionVirtualColumn.getParsedExpression().get(); - if (expr instanceof TimestampFloorExprMacro.TimestampFloorExpr) { - final TimestampFloorExprMacro.TimestampFloorExpr gran = (TimestampFloorExprMacro.TimestampFloorExpr) expr; - if (gran.getArg().getBindingIfIdentifier() != null) { - return gran.getGranularity(); + return fromExpr(((ExpressionVirtualColumn) virtualColumn).getParsedExpression().get()); + } + return null; + } + + private static Granularity fromExpr(Expr expr) + { + String identifier = expr.getIdentifierIfIdentifier(); + if (identifier != null) { + return identifier.equals(ColumnHolder.TIME_COLUMN_NAME) + ? Granularities.NONE + : null; + } + + if (expr instanceof TimestampFloorExprMacro.TimestampFloorExpr) { + final TimestampFloorExprMacro.TimestampFloorExpr gran = (TimestampFloorExprMacro.TimestampFloorExpr) expr; + return gran.getGranularity(); + } else if (expr.getExprArgs().isEmpty()) { + return Granularities.ALL; + } else { + Granularity gran = Granularities.ALL; + for (Expr exprArg : expr.getExprArgs()) { + Granularity newGran = fromExpr(exprArg); + if (newGran == null) { + return null; // cannot determine granularity } + gran = gran.isFinerThan(newGran) ? gran : newGran; } + return gran; } - return null; } } diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java index ba9ba02feb9e..7cf7bfc6c1a0 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java @@ -26,6 +26,7 @@ import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.jsontype.TypeSerializer; import com.google.common.base.Preconditions; +import org.apache.druid.error.DruidException; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; @@ -40,6 +41,15 @@ import javax.annotation.Nullable; import java.io.IOException; +import java.time.Instant; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.zone.ZoneOffsetTransition; +import java.time.zone.ZoneRules; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * PeriodGranularity buckets data based on any custom time period @@ -216,6 +226,129 @@ public String toString() '}'; } + /** + * Returns true if this granularity can be mapped to the given granularity. For example: + *
  • Period('PT1H') in UTC can be mapped to Period('P1D') in UTC
    • + *
  • Period('PT1H') in America/Los_Angeles can be mapped to Period('PT1H') in UTC
    • + *
  • Period('P1D') in America/Los_Angeles cannot be mapped to Period('P1D') in UTC
    • + */ + public boolean canBeMappedTo(PeriodGranularity gran) + { + if (hasOrigin || gran.hasOrigin) { + return false; + } + + if (getTimeZone().equals(gran.getTimeZone())) { + int periodMonths = period.getYears() * 12 + period.getMonths(); + int granMonths = gran.period.getYears() * 12 + gran.period.getMonths(); + if (granMonths == 0 && periodMonths != 0) { + return false; + } + + int periodStandardSeconds = getStandardSecondsOrThrow(period.withYears(0).withMonths(0)); + int granStandardSeconds = getStandardSecondsOrThrow(gran.period.withYears(0).withMonths(0)); + if (granMonths != 0 && periodMonths == 0) { + // if gran month is set, we require it not have week/day/hour/minute/second, and period can be mapped to day + // this is for simplicity, it's possible some period can be mapped to gran, but we don't support it + return granStandardSeconds == 0 && (3600 * 24) % periodStandardSeconds == 0; + } else if (granMonths != 0 && periodMonths != 0) { + return granMonths % periodMonths == 0 + && granStandardSeconds == 0 + && periodStandardSeconds == 0; + } else { + // both periods have zero months + return granStandardSeconds % periodStandardSeconds == 0; + } + } + + if (getStandardSeconds(period).isEmpty()) { + return false; + } + int standardSeconds = getStandardSecondsOrThrow(period); + // if timezone doesn't match, and both periods are in whole seconds, we'd try to map them to UTC + if (standardSeconds != getUtcMappablePeriodSecondsOrThrow()) { + // the period cannot be mapped to UTC with the same period + return false; + } + if (gran.period.getYears() != 0 || gran.period.getMonths() != 0) { + return getStandardSecondsOrThrow(gran.period.withYears(0).withMonths(0)) == 0; + } else { + return gran.getUtcMappablePeriodSecondsOrThrow() % standardSeconds == 0; + } + } + + /** + * Returns the maximum possible period seconds that this granularity can be mapped to UTC. + *

    + * Throws {@link DruidException} if the period cannot be mapped to whole seconds, i.e. it has years or months, or milliseconds. + */ + public int getUtcMappablePeriodSecondsOrThrow() + { + int periodSeconds = PeriodGranularity.getStandardSecondsOrThrow(period); + + if (ISOChronology.getInstanceUTC().getZone().equals(getTimeZone())) { + return periodSeconds; + } + ZoneRules rules = ZoneId.of(getTimeZone().getID()).getRules(); + Set offsets = Stream.concat( + Stream.of(rules.getStandardOffset(Instant.now())), + rules.getTransitions() + .stream() + .filter(t -> t.getInstant().isAfter(Instant.EPOCH)) // timezone transitions before epoch are patchy + .map(ZoneOffsetTransition::getOffsetBefore) + ).map(ZoneOffset::getTotalSeconds).collect(Collectors.toSet()); + + if (offsets.isEmpty()) { + // no offsets + return periodSeconds; + } + + if (offsets.stream().allMatch(o -> o % periodSeconds == 0)) { + // all offsets are multiples of the period + return periodSeconds; + } else if (periodSeconds % 3600 == 0 && offsets.stream().allMatch(o -> o % 3600 == 0)) { + // fall back to hour if period is a multiple of hour and all offsets are multiples of hour + return 3600; + } else if (periodSeconds % 60 == 0 && offsets.stream().allMatch(o -> o % 60 == 0)) { + // fall back to minute if period is a multiple of minute and all offsets are multiples of minute + return 60; + } else { + // default to second + return 1; + } + } + + /** + * Returns the standard whole seconds for the given period. + *

    + * Throws {@link DruidException} if the period cannot be mapped to whole seconds, i.e. it has years or months, or milliseconds. + */ + public static Integer getStandardSecondsOrThrow(Period period) + { + Optional s = getStandardSeconds(period); + if (s.isPresent()) { + return s.get(); + } else { + throw DruidException.defensive("Period[%s] cannot be converted to standard whole seconds", period); + } + } + + /** + * Returns the standard whole seconds for the given period. + *

    + * Returns empty if the period cannot be mapped to whole seconds, i.e. it has years or months, or milliseconds. + */ + public static Optional getStandardSeconds(Period period) + { + if (period.getYears() == 0 && period.getMonths() == 0) { + long millis = period.toStandardDuration().getMillis(); + return millis % 1000 == 0 + ? Optional.of((int) (millis / 1000)) + : Optional.empty(); + } + return Optional.empty(); + } + private static boolean isCompoundPeriod(Period period) { int[] values = period.getValues(); diff --git a/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java b/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java index 1ec567f97b0b..906ac79b8a07 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java +++ b/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java @@ -25,6 +25,7 @@ import javax.annotation.Nullable; import java.util.Arrays; +import java.util.List; import java.util.Objects; /** @@ -82,6 +83,12 @@ public BindingAnalysis analyzeInputs() .withScalarArguments(ImmutableSet.of(left, right)); } + @Override + public List getExprArgs() + { + return Arrays.asList(left, right); + } + @Nullable @Override public ExpressionType getOutputType(InputBindingInspector inspector) diff --git a/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java b/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java index 6d44542171a0..e10031a02da0 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java @@ -33,6 +33,7 @@ import java.math.BigInteger; import java.nio.ByteBuffer; import java.util.Arrays; +import java.util.List; import java.util.Objects; /** @@ -93,6 +94,12 @@ public final BindingAnalysis analyzeInputs() return BindingAnalysis.EMTPY; } + @Override + public List getExprArgs() + { + return List.of(); + } + @Override public boolean canVectorize(InputBindingInspector inspector) { @@ -130,9 +137,10 @@ public ExprVectorProcessor asVectorProcessor(VectorInputBindingInspector { return VectorProcessors.constant(value, inspector.getMaxVectorSize(), outputType); } + /** * Constant expression based on a concreate ExprEval. - * + *

    * Not multi-thread safe. */ @NotThreadSafe diff --git a/processing/src/main/java/org/apache/druid/math/expr/Expr.java b/processing/src/main/java/org/apache/druid/math/expr/Expr.java index 4347ac7af07e..284c726ce090 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/Expr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/Expr.java @@ -158,6 +158,8 @@ default String getBindingIfIdentifier() */ BindingAnalysis analyzeInputs(); + List getExprArgs(); + /** * Given an {@link InputBindingInspector}, compute what the output {@link ExpressionType} will be for this expression. * diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java b/processing/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java index 40d8ba1112c4..cf12fe1f04ee 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java @@ -94,7 +94,6 @@ public List getMacros() * * @param functionName function name * @param args function arguments - * * @return expr for this function call, or null */ @Nullable @@ -119,6 +118,12 @@ public interface ExprMacro extends NamedFunction public interface ExprMacroFunctionExpr extends Expr { List getArgs(); + + @Override + default List getExprArgs() + { + return getArgs(); + } } /** @@ -145,12 +150,6 @@ protected BaseMacroFunctionExpr(final ExprMacro macro, final List macroArg analyzeInputsSupplier = Suppliers.memoize(this::supplyAnalyzeInputs); } - @Override - public List getArgs() - { - return args; - } - @Override public String stringify() { @@ -175,6 +174,18 @@ public BindingAnalysis analyzeInputs() return analyzeInputsSupplier.get(); } + @Override + public List getArgs() + { + return args; + } + + @Override + public List getExprArgs() + { + return args; + } + @Override public boolean canVectorize(InputBindingInspector inspector) { diff --git a/processing/src/main/java/org/apache/druid/math/expr/FunctionalExpr.java b/processing/src/main/java/org/apache/druid/math/expr/FunctionalExpr.java index 02457e992a3a..55eb65a1fce5 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/FunctionalExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/FunctionalExpr.java @@ -46,7 +46,7 @@ final class FunctionalExpr * evaluated. */ @SuppressWarnings("ClassName") -class FunctionExpr implements Expr + class FunctionExpr implements Expr { final Function function; final ImmutableList args; @@ -166,6 +166,12 @@ public BindingAnalysis analyzeInputs() .withArrayOutput(function.hasArrayOutput()); } + @Override + public List getExprArgs() + { + return args; + } + @Override public ExpressionType getOutputType(InputBindingInspector inspector) { @@ -293,6 +299,12 @@ public BindingAnalysis analyzeInputs() return bindingAnalysis; } + @Override + public List getExprArgs() + { + return argsExpr; + } + @Nullable @Override public ExpressionType getOutputType(InputBindingInspector inspector) diff --git a/processing/src/main/java/org/apache/druid/math/expr/IdentifierExpr.java b/processing/src/main/java/org/apache/druid/math/expr/IdentifierExpr.java index 137fd93f5010..8cad56465ed1 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/IdentifierExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/IdentifierExpr.java @@ -28,6 +28,7 @@ import org.apache.druid.segment.column.ColumnType; import javax.annotation.Nullable; +import java.util.List; import java.util.Objects; /** @@ -118,6 +119,13 @@ public BindingAnalysis analyzeInputs() return new BindingAnalysis(this); } + @Override + public List getExprArgs() + { + // identifier has no arguments + return List.of(); + } + @Override public ExpressionType getOutputType(InputBindingInspector inspector) { diff --git a/processing/src/main/java/org/apache/druid/math/expr/LambdaExpr.java b/processing/src/main/java/org/apache/druid/math/expr/LambdaExpr.java index 2dc39dc24d18..da50969539fa 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/LambdaExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/LambdaExpr.java @@ -123,6 +123,12 @@ public BindingAnalysis analyzeInputs() return bodyDetails.removeLambdaArguments(lambdaArgs); } + @Override + public List getExprArgs() + { + return args.stream().map(i -> (Expr) i).collect(Collectors.toList()); + } + @Override public ExpressionType getOutputType(InputBindingInspector inspector) { diff --git a/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java b/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java index 97aad5add51b..76f625df3dc8 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java @@ -28,6 +28,7 @@ import javax.annotation.Nullable; import java.math.BigInteger; +import java.util.List; import java.util.Objects; @@ -72,6 +73,12 @@ public BindingAnalysis analyzeInputs() return expr.analyzeInputs().withScalarArguments(ImmutableSet.of(expr)); } + @Override + public List getExprArgs() + { + return List.of(expr); + } + @Nullable @Override public ExpressionType getOutputType(InputBindingInspector inspector) diff --git a/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java b/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java index 115c9eed397d..45b5cdabad04 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java +++ b/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java @@ -396,6 +396,12 @@ public BindingAnalysis analyzeInputs() return originalExpr.analyzeInputs(); } + @Override + public List getExprArgs() + { + return List.of(originalExpr); + } + @Override public boolean isLiteral() { diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java index 3c5102ae7a2c..1d6ce2439129 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java @@ -56,14 +56,19 @@ public Expr apply(final List args) } @VisibleForTesting - static class TimestampCeilExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr + public static class TimestampCeilExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr { private final Granularity granularity; TimestampCeilExpr(final TimestampCeilExprMacro macro, final List args) { super(macro, args); - this.granularity = getGranularity(this, args, InputBindings.nilBindings()); + this.granularity = computeGranularity(this, args, InputBindings.nilBindings()); + } + + public Granularity getGranularity() + { + return granularity; } @Nonnull @@ -113,7 +118,7 @@ public int hashCode() } } - private static PeriodGranularity getGranularity( + private static PeriodGranularity computeGranularity( final Expr expr, final List args, final Expr.ObjectBinding bindings @@ -140,7 +145,7 @@ static class TimestampCeilDynamicExpr extends ExprMacroTable.BaseScalarMacroFunc @Override public ExprEval eval(final ObjectBinding bindings) { - final PeriodGranularity granularity = getGranularity(this, args, bindings); + final PeriodGranularity granularity = computeGranularity(this, args, bindings); long argTime = args.get(0).eval(bindings).asLong(); long bucketStartTime = granularity.bucketStart(argTime); if (argTime == bucketStartTime) { diff --git a/processing/src/main/java/org/apache/druid/segment/projections/Projections.java b/processing/src/main/java/org/apache/druid/segment/projections/Projections.java index 472588860179..5e1769efb1e5 100644 --- a/processing/src/main/java/org/apache/druid/segment/projections/Projections.java +++ b/processing/src/main/java/org/apache/druid/segment/projections/Projections.java @@ -25,6 +25,7 @@ import org.apache.druid.error.InvalidInput; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.java.util.common.granularity.PeriodGranularity; import org.apache.druid.query.QueryContexts; import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.query.filter.Filter; @@ -424,13 +425,19 @@ public static ProjectionMatchBuilder matchQueryVirtualColumn( // virtual column and underlying expression itself, but this will do for now final Granularity virtualGranularity = Granularities.fromVirtualColumn(queryVirtualColumn); if (virtualGranularity != null) { - if (virtualGranularity.isFinerThan(projection.getEffectiveGranularity())) { - return null; - } // same granularity, replace virtual column directly by remapping it to the physical column if (projection.getEffectiveGranularity().equals(virtualGranularity)) { return matchBuilder.remapColumn(queryVirtualColumn.getOutputName(), ColumnHolder.TIME_COLUMN_NAME) .addReferencedPhysicalColumn(ColumnHolder.TIME_COLUMN_NAME); + } else if (virtualGranularity instanceof PeriodGranularity + && projection.getEffectiveGranularity() instanceof PeriodGranularity) { + PeriodGranularity virtualGranularityPeriod = (PeriodGranularity) virtualGranularity; + PeriodGranularity projectionGranularityPeriod = (PeriodGranularity) projection.getEffectiveGranularity(); + if (!projectionGranularityPeriod.canBeMappedTo(virtualGranularityPeriod)) { + return null; + } + } else if (virtualGranularity.isFinerThan(projection.getEffectiveGranularity())) { + return null; } return matchBuilder.addReferencedPhysicalColumn(ColumnHolder.TIME_COLUMN_NAME); } else { diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java index fc2e7ddcc458..d248efd9d959 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java @@ -129,7 +129,7 @@ private ExpressionVirtualColumn( parsedExpression, outputType ); - this.expressionAnalysis = Suppliers.memoize(() -> parsedExpression.get().analyzeInputs()); + this.expressionAnalysis = Suppliers.memoize(parsedExpression.get()::analyzeInputs); this.cacheKey = makeCacheKeySupplier(); } diff --git a/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java b/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java index a885b2f6df1b..00a07e7219d0 100644 --- a/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java +++ b/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java @@ -57,6 +57,7 @@ import java.util.TimeZone; /** + * */ public class QueryGranularityTest extends InitializedNullHandlingTest { @@ -1044,7 +1045,7 @@ public void testToVirtualColumn() column = Granularities.toVirtualColumn(hourWithOrigin, Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME); Assert.assertEquals(ColumnHolder.TIME_COLUMN_NAME, column.getExpression()); column = Granularities.toVirtualColumn(hourWithTz, Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME); - Assert.assertEquals(ColumnHolder.TIME_COLUMN_NAME, column.getExpression()); + Assert.assertEquals("timestamp_floor(__time,'PT1H')", column.getExpression()); column = Granularities.toVirtualColumn(duration, Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME); Assert.assertEquals(ColumnHolder.TIME_COLUMN_NAME, column.getExpression()); column = Granularities.toVirtualColumn(Granularities.NONE, Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME); @@ -1070,6 +1071,18 @@ public void testFromVirtualColumn() ColumnType.LONG, TestExprMacroTable.INSTANCE ); + ExpressionVirtualColumn hourlyPacificTime = new ExpressionVirtualColumn( + "v0", + "timestamp_floor(__gran, 'PT1H', null, 'America/Los_Angeles')", + ColumnType.LONG, + TestExprMacroTable.INSTANCE + ); + ExpressionVirtualColumn hourlyIndianTime = new ExpressionVirtualColumn( + "v0", + "timestamp_floor(__gran, 'PT1H', null, 'Asia/Kolkata')", + ColumnType.LONG, + TestExprMacroTable.INSTANCE + ); ExpressionVirtualColumn ceilHour = new ExpressionVirtualColumn( "v0", "timestamp_ceil(__time, 'PT1M')", @@ -1078,7 +1091,7 @@ public void testFromVirtualColumn() ); ExpressionVirtualColumn floorWithExpression = new ExpressionVirtualColumn( "v0", - "timestamp_floor(timestamp_parse(timestamp,null,'UTC'), 'PT1M')", + "timestamp_floor(timestamp_parse(__time,null,'UTC'), 'PT1M')", ColumnType.LONG, TestExprMacroTable.INSTANCE ); @@ -1097,8 +1110,16 @@ public void testFromVirtualColumn() Assert.assertEquals(Granularities.HOUR, Granularities.fromVirtualColumn(hourly)); Assert.assertEquals(Granularities.DAY, Granularities.fromVirtualColumn(day)); Assert.assertEquals(Granularities.HOUR, Granularities.fromVirtualColumn(hourlyNonstandardTime)); - Assert.assertNull(Granularities.fromVirtualColumn(ceilHour)); - Assert.assertNull(Granularities.fromVirtualColumn(floorWithExpression)); + Assert.assertEquals( + new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.forID("America/Los_Angeles")), + Granularities.fromVirtualColumn(hourlyPacificTime) + ); + Assert.assertEquals( + new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.forID("Asia/Kolkata")), + Granularities.fromVirtualColumn(hourlyIndianTime) + ); + Assert.assertEquals(Granularities.NONE, Granularities.fromVirtualColumn(ceilHour)); + Assert.assertEquals(Granularities.MINUTE, Granularities.fromVirtualColumn(floorWithExpression)); final DateTime origin = DateTimes.of("2012-01-02T05:00:00.000-08:00"); final DateTimeZone tz = DateTimes.inferTzFromString("America/Los_Angeles"); final Granularity minuteWithTz = new PeriodGranularity(new Period("PT1M"), null, tz); @@ -1107,6 +1128,37 @@ public void testFromVirtualColumn() Assert.assertEquals(minuteWithOrigin, Granularities.fromVirtualColumn(floorWithOriginTimezone)); } + @Test + public void testFromVirtualColumnExtra() + { + ExpressionVirtualColumn formatFloor = new ExpressionVirtualColumn( + "v0", + "timestamp_format(timestamp_floor(__time, 'PT1H', null,'America/Los_Angeles'), 'yyyy-MM-dd')", + ColumnType.LONG, + TestExprMacroTable.INSTANCE + ); + Assert.assertEquals( + new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.forID("America/Los_Angeles")), + Granularities.fromVirtualColumn(formatFloor) + ); + + ExpressionVirtualColumn concatFormatFloor = new ExpressionVirtualColumn( + "v0", + "concat(a, timestamp_format(timestamp_floor(__time, 'PT1H', null,'America/Los_Angeles'), 'yyyy-MM-dd'))", + ColumnType.LONG, + TestExprMacroTable.INSTANCE + ); + Assert.assertNull(Granularities.fromVirtualColumn(concatFormatFloor)); + + ExpressionVirtualColumn concatFields = new ExpressionVirtualColumn( + "v0", + "concat(a, b)", + ColumnType.LONG, + TestExprMacroTable.INSTANCE + ); + Assert.assertNull(Granularities.fromVirtualColumn(concatFields)); + } + private void assertBucketStart(final Granularity granularity, final DateTime in, final DateTime expectedInProperTz) { Assert.assertEquals( diff --git a/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java b/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java new file mode 100644 index 000000000000..a51bee9a0c42 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java @@ -0,0 +1,136 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import org.apache.druid.java.util.common.granularity.PeriodGranularity; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.Period; +import org.junit.Test; +import org.junit.jupiter.api.Assertions; + +public class PeriodGranularityTest +{ + PeriodGranularity UTC_PT1H = new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.UTC); + PeriodGranularity UTC_PT1M = new PeriodGranularity(new Period("PT1M"), null, DateTimeZone.UTC); + + DateTimeZone PACIFIC_TZ = DateTimeZone.forID("America/Los_Angeles"); + DateTimeZone INDIAN_TZ = DateTimeZone.forID("Asia/Kolkata"); + + @Test + public void testCanBeMappedTo_sameTimeZone() + { + Assertions.assertTrue(UTC_PT1M.canBeMappedTo(UTC_PT1H)); + + PeriodGranularity pacificPT2H = new PeriodGranularity(new Period("PT2H"), null, PACIFIC_TZ); + Assertions.assertFalse(pacificPT2H.canBeMappedTo(new PeriodGranularity(new Period("PT20M"), null, PACIFIC_TZ))); + Assertions.assertTrue(pacificPT2H.canBeMappedTo(new PeriodGranularity(new Period("PT6H"), null, PACIFIC_TZ))); + + PeriodGranularity pacificP1D = new PeriodGranularity(new Period("P1D"), null, PACIFIC_TZ); + Assertions.assertFalse(pacificP1D.canBeMappedTo(new PeriodGranularity(new Period("PT1H"), null, PACIFIC_TZ))); + Assertions.assertTrue(pacificP1D.canBeMappedTo(new PeriodGranularity(new Period("P1M"), null, PACIFIC_TZ))); + + PeriodGranularity pacificP2D = new PeriodGranularity(new Period("P2D"), null, PACIFIC_TZ); + Assertions.assertFalse(pacificP2D.canBeMappedTo(new PeriodGranularity(new Period("P1W"), null, PACIFIC_TZ))); + Assertions.assertTrue(pacificP2D.canBeMappedTo(new PeriodGranularity(new Period("P2W"), null, PACIFIC_TZ))); + + // some extra tests for different month/week/day combo + PeriodGranularity pacificPT1W2D = new PeriodGranularity(new Period("P1W").withDays(2), null, PACIFIC_TZ); + Assertions.assertFalse(pacificPT1W2D.canBeMappedTo(new PeriodGranularity(new Period("P2M"), null, PACIFIC_TZ))); + Assertions.assertFalse(pacificPT1W2D.canBeMappedTo(new PeriodGranularity(new Period("P2Y"), null, PACIFIC_TZ))); + Assertions.assertTrue(pacificPT1W2D.canBeMappedTo(pacificPT1W2D)); + Assertions.assertTrue(pacificPT1W2D.canBeMappedTo(new PeriodGranularity( + new Period("P2W").withDays(4), + null, + PACIFIC_TZ + ))); + + Assertions.assertTrue(pacificP1D.canBeMappedTo(pacificPT1W2D)); + Assertions.assertTrue(pacificPT2H.canBeMappedTo(new PeriodGranularity( + new Period("P1D").withHours(2), + null, + PACIFIC_TZ + ))); + } + + @Test + public void testCanBeMappedTo_sameHourlyAlignWithUtc() + { + Assertions.assertTrue(UTC_PT1H.canBeMappedTo(new PeriodGranularity(new Period("PT1H"), null, PACIFIC_TZ))); + Assertions.assertTrue(UTC_PT1H.canBeMappedTo(new PeriodGranularity(new Period("PT3H"), null, PACIFIC_TZ))); + } + + @Test + public void testCanBeMappedTo_sameMinuteAlignWithUtc() + { + Assertions.assertFalse(UTC_PT1H.canBeMappedTo(new PeriodGranularity(new Period("PT2H"), null, INDIAN_TZ))); + Assertions.assertTrue(UTC_PT1M.canBeMappedTo(new PeriodGranularity(new Period("PT1M"), null, PACIFIC_TZ))); + } + + @Test + public void testCanBeMappedTo_withOrigin() + { + Assertions.assertFalse(new PeriodGranularity( + new Period("PT1H"), + new DateTime(), + DateTimeZone.UTC + ).canBeMappedTo(new PeriodGranularity(new Period("PT2H"), null, DateTimeZone.UTC))); + + Assertions.assertFalse(UTC_PT1H.canBeMappedTo(new PeriodGranularity( + new Period("PT1H"), + new DateTime(), + DateTimeZone.UTC + ))); + } + + @Test + public void testCanBeMappedTo_differentTimeZone() + { + // In theory pacificPT1M should be able to map to UTC_PT1M, but we don't support this for simplicity. + PeriodGranularity pacificPT1H = new PeriodGranularity(new Period("PT1H"), null, PACIFIC_TZ); + Assertions.assertTrue(pacificPT1H.canBeMappedTo(UTC_PT1H)); + Assertions.assertTrue(pacificPT1H.canBeMappedTo(new PeriodGranularity( + new Period("PT3H"), + null, + DateTimeZone.forID("America/New_York") + ))); + Assertions.assertTrue(pacificPT1H.canBeMappedTo(new PeriodGranularity( + new Period("P1M"), + null, + DateTimeZone.UTC + ))); + + Assertions.assertFalse(pacificPT1H.canBeMappedTo(new PeriodGranularity( + new Period("PT1H"), + null, + INDIAN_TZ + ))); + Assertions.assertFalse(pacificPT1H.canBeMappedTo(new PeriodGranularity( + new Period("P1D"), + null, + INDIAN_TZ + ))); + Assertions.assertFalse(new PeriodGranularity( + new Period("P1D"), + null, + PACIFIC_TZ + ).canBeMappedTo(new PeriodGranularity(new Period("P1D"), null, DateTimeZone.UTC))); + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java b/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java index 7f9e82afb2e7..cd6363313079 100644 --- a/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java +++ b/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java @@ -41,8 +41,10 @@ import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.Pair; import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.PeriodGranularity; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.java.util.common.io.Closer; +import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.DefaultQueryMetrics; import org.apache.druid.query.DruidProcessingConfig; import org.apache.druid.query.Druids; @@ -53,10 +55,12 @@ import org.apache.druid.query.aggregation.CountAggregatorFactory; import org.apache.druid.query.aggregation.DoubleSumAggregatorFactory; import org.apache.druid.query.aggregation.FloatSumAggregatorFactory; +import org.apache.druid.query.aggregation.LongMaxAggregatorFactory; import org.apache.druid.query.aggregation.LongSumAggregatorFactory; import org.apache.druid.query.aggregation.firstlast.last.LongLastAggregatorFactory; import org.apache.druid.query.dimension.DefaultDimensionSpec; import org.apache.druid.query.expression.TestExprMacroTable; +import org.apache.druid.query.expression.TimestampFloorExprMacro; import org.apache.druid.query.filter.EqualityFilter; import org.apache.druid.query.groupby.GroupByQuery; import org.apache.druid.query.groupby.GroupByQueryConfig; @@ -67,6 +71,7 @@ import org.apache.druid.query.groupby.ResultRow; import org.apache.druid.query.groupby.orderby.DefaultLimitSpec; import org.apache.druid.query.groupby.orderby.OrderByColumnSpec; +import org.apache.druid.query.groupby.orderby.OrderByColumnSpec.Direction; import org.apache.druid.query.ordering.StringComparators; import org.apache.druid.query.timeseries.TimeseriesQuery; import org.apache.druid.query.timeseries.TimeseriesQueryEngine; @@ -81,6 +86,8 @@ import org.apache.druid.segment.virtual.NestedFieldVirtualColumn; import org.apache.druid.testing.InitializedNullHandlingTest; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.Period; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Assume; @@ -107,7 +114,10 @@ public class CursorFactoryProjectionTest extends InitializedNullHandlingTest { private static final Closer CLOSER = Closer.create(); - static final DateTime TIMESTAMP = Granularities.DAY.bucket(DateTimes.nowUtc()).getStart(); + // Set a fixed time, when IST is 5 hours 30 minutes ahead of UTC, and PDT is 7 hours behind UTC. + static final DateTime UTC_MIDNIGHT = Granularities.DAY.bucket(DateTimes.of("2025-08-13")).getStart(); + static final DateTime UTC_01H = UTC_MIDNIGHT.plusHours(1); + static final DateTime UTC_01H31M = UTC_MIDNIGHT.plusHours(1).plusMinutes(31); static final RowSignature ROW_SIGNATURE = RowSignature.builder() .add("a", ColumnType.STRING) @@ -123,49 +133,49 @@ public static List makeRows(List dimensions) return Arrays.asList( new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP, + UTC_MIDNIGHT, dimensions, Arrays.asList("a", "aa", 1L, 1.0, null, Map.of("x", "a", "y", 1L, "z", 1.0)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusMinutes(2), + UTC_MIDNIGHT.plusMinutes(2), dimensions, Arrays.asList("a", "bb", 1L, 1.1, 1.1f, Map.of("x", "a", "y", 1L, "z", 1.1)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusMinutes(4), + UTC_MIDNIGHT.plusMinutes(4), dimensions, Arrays.asList("a", "cc", 2L, 2.2, 2.2f, Map.of("x", "a", "y", 2L, "z", 2.2)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusMinutes(6), + UTC_MIDNIGHT.plusMinutes(6), dimensions, Arrays.asList("b", "aa", 3L, 3.3, 3.3f, Map.of("x", "b", "y", 3L, "z", 3.3)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusMinutes(8), + UTC_MIDNIGHT.plusMinutes(8), dimensions, Arrays.asList("b", "aa", 4L, 4.4, 4.4f, Map.of("x", "b", "y", 4L, "z", 4.4)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusMinutes(10), + UTC_MIDNIGHT.plusMinutes(10), dimensions, Arrays.asList("b", "bb", 5L, 5.5, 5.5f, Map.of("x", "b", "y", 5L, "z", 5.5)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusHours(1), + UTC_01H, dimensions, Arrays.asList("a", "aa", 1L, 1.1, 1.1f, Map.of("x", "a", "y", 1L, "z", 1.1)) ), new ListBasedInputRow( ROW_SIGNATURE, - TIMESTAMP.plusHours(1).plusMinutes(1), + UTC_01H31M, dimensions, Arrays.asList("a", "dd", 2L, 2.2, 2.2f, Map.of("x", "a", "y", 2L, "z", 2.2)) ) @@ -275,8 +285,18 @@ public static List makeRows(List dimensions) .build(), AggregateProjectionSpec.builder("a_concat_b_d_plus_f_sum_c") .virtualColumns( - new ExpressionVirtualColumn("__vc2", "d + e", ColumnType.LONG, TestExprMacroTable.INSTANCE), - new ExpressionVirtualColumn("__vc3", "concat(a, b)", ColumnType.STRING, TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn( + "__vc2", + "d + e", + ColumnType.LONG, + TestExprMacroTable.INSTANCE + ), + new ExpressionVirtualColumn( + "__vc3", + "concat(a, b)", + ColumnType.STRING, + TestExprMacroTable.INSTANCE + ) ) .groupingColumns(new LongDimensionSchema("__vc2"), new StringDimensionSchema("__vc3")) .aggregators(new LongSumAggregatorFactory("sum_c", "c")) @@ -290,10 +310,10 @@ public static List makeRows(List dimensions) new LongDimensionSchema("__gran"), new StringDimensionSchema("a") ) - .aggregators( - new CountAggregatorFactory("chocula"), - new LongSumAggregatorFactory("sum_c", "sum_c") - ) + .aggregators( + new CountAggregatorFactory("chocula"), + new LongSumAggregatorFactory("sum_c", "sum_c") + ) .build(), AggregateProjectionSpec.builder("afoo") .virtualColumns( @@ -305,7 +325,10 @@ public static List makeRows(List dimensions) ) ) .groupingColumns(new StringDimensionSchema("afoo")) - .aggregators(new LongSumAggregatorFactory("sum_c", "sum_c")) + .aggregators( + new LongSumAggregatorFactory("sum_c", "sum_c"), + new LongMaxAggregatorFactory("max_c", "max_c") + ) .build() ); @@ -328,14 +351,13 @@ public static List makeRows(List dimensions) ROLLUP_PROJECTIONS.stream() .map( projection -> - AggregateProjectionSpec.builder(projection) - .groupingColumns( - projection.getGroupingColumns() - .stream() - .map(x -> new AutoTypeColumnSchema(x.getName(), null)) - .collect(Collectors.toList()) - ) - .build() + AggregateProjectionSpec + .builder(projection) + .groupingColumns(projection.getGroupingColumns() + .stream() + .map(x -> new AutoTypeColumnSchema(x.getName(), null)) + .collect(Collectors.toList())) + .build() ) .collect(Collectors.toList()); @@ -366,6 +388,7 @@ public static Collection constructorFeeder() ) ); final AggregatorFactory[] rollupAggs = new AggregatorFactory[]{ + new LongMaxAggregatorFactory("max_c", "c"), new LongSumAggregatorFactory("sum_c", "c"), new DoubleSumAggregatorFactory("sum_d", "d"), new FloatSumAggregatorFactory("sum_e", "e") @@ -413,7 +436,11 @@ public static Collection constructorFeeder() } } if (incremental) { - IncrementalIndex index = CLOSER.register(makeBuilder(dims, autoSchema, writeNullColumns).buildIncrementalIndex()); + IncrementalIndex index = CLOSER.register(makeBuilder( + dims, + autoSchema, + writeNullColumns + ).buildIncrementalIndex()); IncrementalIndex rollupIndex = CLOSER.register( makeRollupBuilder(rollupDims, rollupAggs, autoSchema).buildIncrementalIndex() ); @@ -427,7 +454,11 @@ public static Collection constructorFeeder() autoSchema }); } else { - QueryableIndex index = CLOSER.register(makeBuilder(dims, autoSchema, writeNullColumns).buildMMappedIndex()); + QueryableIndex index = CLOSER.register(makeBuilder( + dims, + autoSchema, + writeNullColumns + ).buildMMappedIndex()); QueryableIndex rollupIndex = CLOSER.register( makeRollupBuilder(rollupDims, rollupAggs, autoSchema).buildMMappedIndex() ); @@ -566,8 +597,8 @@ public void testProjectionSelectionTwoDimsVirtual() .setLimitSpec( new DefaultLimitSpec( Arrays.asList( - new OrderByColumnSpec("a", OrderByColumnSpec.Direction.ASCENDING, StringComparators.LEXICOGRAPHIC), - new OrderByColumnSpec("v0", OrderByColumnSpec.Direction.ASCENDING, StringComparators.LEXICOGRAPHIC) + new OrderByColumnSpec("a", Direction.ASCENDING, StringComparators.LEXICOGRAPHIC), + new OrderByColumnSpec("v0", Direction.ASCENDING, StringComparators.LEXICOGRAPHIC) ), 10 ) @@ -674,8 +705,8 @@ public void testProjectionSkipContext() query, queryMetrics, List.of( - new Object[]{"a", 7L, Pair.of(TIMESTAMP.plusHours(1).plusMinutes(1).getMillis(), 2L)}, - new Object[]{"b", 12L, Pair.of(TIMESTAMP.plusMinutes(10).getMillis(), 5L)} + new Object[]{"a", 7L, Pair.of(UTC_01H31M.getMillis(), 2L)}, + new Object[]{"b", 12L, Pair.of(UTC_MIDNIGHT.plusMinutes(10).getMillis(), 5L)} ) ); } @@ -702,8 +733,8 @@ public void testProjectionSingleDim() query, queryMetrics, List.of( - new Object[]{"a", 7L, Pair.of(TIMESTAMP.plusHours(1).plusMinutes(1).getMillis(), 2L)}, - new Object[]{"b", 12L, Pair.of(TIMESTAMP.plusMinutes(10).getMillis(), 5L)} + new Object[]{"a", 7L, Pair.of(UTC_01H31M.getMillis(), 2L)}, + new Object[]{"b", 12L, Pair.of(UTC_MIDNIGHT.plusMinutes(10).getMillis(), 5L)} ) ); } @@ -757,7 +788,7 @@ public void testProjectionSingleDimFilter() query, queryMetrics, Collections.singletonList( - new Object[]{"a", 7L, Pair.of(TIMESTAMP.plusHours(1).plusMinutes(1).getMillis(), 2L)} + new Object[]{"a", 7L, Pair.of(UTC_01H31M.getMillis(), 2L)} ) ); } @@ -886,9 +917,9 @@ public void testQueryGranularityFitsProjectionGranularity() query, queryMetrics, makeArrayResultSet( - new Object[]{TIMESTAMP.getMillis(), "a", 4L}, - new Object[]{TIMESTAMP.getMillis(), "b", 12L}, - new Object[]{TIMESTAMP.plusHours(1).getMillis(), "a", 3L} + new Object[]{UTC_MIDNIGHT.getMillis(), "a", 4L}, + new Object[]{UTC_MIDNIGHT.getMillis(), "b", 12L}, + new Object[]{UTC_01H.getMillis(), "a", 3L} ) ); } @@ -923,15 +954,106 @@ public void testQueryGranularityFitsProjectionGranularityNotTimeOrdered() query, queryMetrics, makeArrayResultSet( - new Object[]{TIMESTAMP.getMillis(), "aa", 8L}, - new Object[]{TIMESTAMP.getMillis(), "bb", 6L}, - new Object[]{TIMESTAMP.getMillis(), "cc", 2L}, - new Object[]{TIMESTAMP.plusHours(1).getMillis(), "aa", 1L}, - new Object[]{TIMESTAMP.plusHours(1).getMillis(), "dd", 2L} + new Object[]{UTC_MIDNIGHT.getMillis(), "aa", 8L}, + new Object[]{UTC_MIDNIGHT.getMillis(), "bb", 6L}, + new Object[]{UTC_MIDNIGHT.getMillis(), "cc", 2L}, + new Object[]{UTC_01H.getMillis(), "aa", 1L}, + new Object[]{UTC_01H.getMillis(), "dd", 2L} ) ); } + @Test + public void testQueryGranularityFitsProjectionGranularityWithTimeZone() + { + final GroupByQuery.Builder queryBuilder = + GroupByQuery.builder() + .setDataSource("test") + .setInterval(Intervals.ETERNITY) + .addAggregator(new LongSumAggregatorFactory("c_sum", "c")); + final ExpectedProjectionGroupBy queryMetrics = new ExpectedProjectionGroupBy("a_hourly_c_sum_with_count_latest"); + + if (segmentSortedByTime) { + queryBuilder.addDimension("a") + .setGranularity(new PeriodGranularity( + new Period("PT1H"), + null, + DateTimeZone.forID("America/Los_Angeles") + )); + } else { + queryBuilder.setGranularity(Granularities.ALL) + .setDimensions( + DefaultDimensionSpec.of("__gran", ColumnType.LONG), + DefaultDimensionSpec.of("a") + ) + .setVirtualColumns(new ExpressionVirtualColumn( + "__gran", + "timestamp_floor(__time,'PT1H',null,'America/Los_Angeles')", + ColumnType.LONG, + new ExprMacroTable(List.of(new TimestampFloorExprMacro())) + )); + } + final GroupByQuery query = queryBuilder.build(); + final CursorBuildSpec buildSpec = GroupingEngine.makeCursorBuildSpec(query, queryMetrics); + + assertCursorProjection(buildSpec, queryMetrics, 3); + + testGroupBy( + query, + queryMetrics, + makeArrayResultSet( + new Object[]{UTC_MIDNIGHT.getMillis(), "a", 4L}, + new Object[]{UTC_MIDNIGHT.getMillis(), "b", 12L}, + new Object[]{UTC_01H.getMillis(), "a", 3L} + ) + ); + } + + @Test + public void testQueryGranularityDoesNotFitProjectionGranularityWithTimeZone() + { + final GroupByQuery.Builder queryBuilder = + GroupByQuery.builder() + .setDataSource("test") + .setInterval(Intervals.ETERNITY) + .addAggregator(new LongSumAggregatorFactory("c_sum", "c")); + final ExpectedProjectionGroupBy queryMetrics = new ExpectedProjectionGroupBy(null); + + if (segmentSortedByTime) { + queryBuilder.addDimension("a") + .setGranularity(new PeriodGranularity( + new Period("PT1H"), + null, + DateTimeZone.forID("Asia/Kolkata") + )); + } else { + queryBuilder.setGranularity(Granularities.ALL) + .setDimensions( + DefaultDimensionSpec.of("__gran", ColumnType.LONG), + DefaultDimensionSpec.of("a") + ) + .setVirtualColumns(new ExpressionVirtualColumn( + "__gran", + "timestamp_floor(__time,'PT1H',null,'Asia/Kolkata')", + ColumnType.LONG, + new ExprMacroTable(List.of(new TimestampFloorExprMacro())) + )); + } + final GroupByQuery query = queryBuilder.build(); + final CursorBuildSpec buildSpec = GroupingEngine.makeCursorBuildSpec(query, queryMetrics); + + assertCursorProjection(buildSpec, queryMetrics, 8); + testGroupBy( + query, + queryMetrics, + makeArrayResultSet( + new Object[]{UTC_MIDNIGHT.minusMinutes(30).getMillis(), "a", 4L}, + new Object[]{UTC_MIDNIGHT.minusMinutes(30).getMillis(), "b", 12L}, + new Object[]{UTC_01H.minusMinutes(30).getMillis(), "a", 1L}, + new Object[]{UTC_01H31M.minusMinutes(1).getMillis(), "a", 2L} + ) + ); + } @Test public void testQueryGranularityLargerProjectionGranularity() @@ -963,8 +1085,8 @@ public void testQueryGranularityLargerProjectionGranularity() query, queryMetrics, makeArrayResultSet( - new Object[]{TIMESTAMP.getMillis(), "a", 7L}, - new Object[]{TIMESTAMP.getMillis(), "b", 12L} + new Object[]{UTC_MIDNIGHT.getMillis(), "a", 7L}, + new Object[]{UTC_MIDNIGHT.getMillis(), "b", 12L} ) ); } @@ -1076,7 +1198,7 @@ public void testTimeseriesQueryAllGranularityCanMatchNonTimeDimProjection() query, queryMetrics, Collections.singletonList( - new Object[]{TIMESTAMP, 19L} + new Object[]{UTC_MIDNIGHT, 19L} ) ); } @@ -1104,7 +1226,7 @@ public void testTimeseriesQueryAllGranularitiesAlwaysRuns() query, queryMetrics, Collections.singletonList( - new Object[]{TIMESTAMP, 19L} + new Object[]{UTC_MIDNIGHT, 19L} ) ); } @@ -1125,7 +1247,8 @@ public void testTimeseriesQueryOrderByNotCompatibleWithProjection() final CursorBuildSpec buildSpec = TimeseriesQueryEngine.makeCursorBuildSpec(query, null); DruidException e = Assert.assertThrows( DruidException.class, - () -> projectionsCursorFactory.makeCursorHolder(buildSpec)); + () -> projectionsCursorFactory.makeCursorHolder(buildSpec) + ); Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory()); Assert.assertEquals("Projection[b_c_sum] specified, but does not satisfy query", e.getMessage()); } @@ -1150,8 +1273,8 @@ public void testTimeseriesQueryGranularityFitsProjectionGranularity() query, queryMetrics, List.of( - new Object[]{TIMESTAMP, 16L}, - new Object[]{TIMESTAMP.plusHours(1), 3L} + new Object[]{UTC_MIDNIGHT, 16L}, + new Object[]{UTC_01H, 3L} ) ); } @@ -1177,7 +1300,7 @@ public void testTimeseriesQueryGranularityAllFitsProjectionGranularityWithSegmen query, queryMetrics, Collections.singletonList( - new Object[]{TIMESTAMP, 19L} + new Object[]{UTC_MIDNIGHT, 19L} ) ); } @@ -1203,7 +1326,7 @@ public void testTimeseriesQueryGranularityAllFitsProjectionGranularityWithNoGrou query, queryMetrics, Collections.singletonList( - new Object[]{TIMESTAMP, 19L} + new Object[]{UTC_MIDNIGHT, 19L} ) ); } @@ -1230,14 +1353,14 @@ public void testTimeseriesQueryGranularityFinerThanProjectionGranularity() query, queryMetrics, List.of( - new Object[]{TIMESTAMP, 1L}, - new Object[]{TIMESTAMP.plusMinutes(2), 1L}, - new Object[]{TIMESTAMP.plusMinutes(4), 2L}, - new Object[]{TIMESTAMP.plusMinutes(6), 3L}, - new Object[]{TIMESTAMP.plusMinutes(8), 4L}, - new Object[]{TIMESTAMP.plusMinutes(10), 5L}, - new Object[]{TIMESTAMP.plusHours(1), 1L}, - new Object[]{TIMESTAMP.plusHours(1).plusMinutes(1), 2L} + new Object[]{UTC_MIDNIGHT, 1L}, + new Object[]{UTC_MIDNIGHT.plusMinutes(2), 1L}, + new Object[]{UTC_MIDNIGHT.plusMinutes(4), 2L}, + new Object[]{UTC_MIDNIGHT.plusMinutes(6), 3L}, + new Object[]{UTC_MIDNIGHT.plusMinutes(8), 4L}, + new Object[]{UTC_MIDNIGHT.plusMinutes(10), 5L}, + new Object[]{UTC_01H, 1L}, + new Object[]{UTC_01H31M, 2L} ) ); } @@ -1275,18 +1398,24 @@ public void testProjectionSingleDimRollupTable() @Test public void testProjectionSingleDimVirtualColumnRollupTable() { + final VirtualColumn vc = new ExpressionVirtualColumn( + "v0", + "concat(a, 'foo')", + ColumnType.STRING, + TestExprMacroTable.INSTANCE + ); final GroupByQuery query = GroupByQuery.builder() .setDataSource("test") .setGranularity(Granularities.ALL) .setInterval(Intervals.ETERNITY) .addDimension("v0") - .setVirtualColumns(new ExpressionVirtualColumn("v0", "concat(a, 'foo')", ColumnType.STRING, TestExprMacroTable.INSTANCE)) + .setVirtualColumns(vc) .addAggregator(new LongSumAggregatorFactory("c_sum", "sum_c")) + .addAggregator(new LongMaxAggregatorFactory("c_c", "max_c")) .build(); - final ExpectedProjectionGroupBy queryMetrics = - new ExpectedProjectionGroupBy("afoo"); + final ExpectedProjectionGroupBy queryMetrics = new ExpectedProjectionGroupBy("afoo"); final CursorBuildSpec buildSpec = GroupingEngine.makeCursorBuildSpec(query, queryMetrics); assertCursorProjection(rollupProjectionsCursorFactory, buildSpec, queryMetrics, 2); @@ -1297,8 +1426,8 @@ public void testProjectionSingleDimVirtualColumnRollupTable() query, queryMetrics, makeArrayResultSet( - new Object[]{"afoo", 7L}, - new Object[]{"bfoo", 12L} + new Object[]{"afoo", 7L, 2L}, + new Object[]{"bfoo", 12L, 5L} ) ); } @@ -1612,7 +1741,7 @@ private static IndexBuilder makeBuilder(DimensionsSpec dimensionsSpec, boolean a IncrementalIndexSchema.builder() .withDimensionsSpec(dimensionsSpec) .withRollup(false) - .withMinTimestamp(TIMESTAMP.getMillis()) + .withMinTimestamp(UTC_MIDNIGHT.getMillis()) .withProjections(autoSchema ? AUTO_PROJECTIONS : PROJECTIONS) .build() ) @@ -1620,20 +1749,25 @@ private static IndexBuilder makeBuilder(DimensionsSpec dimensionsSpec, boolean a .rows(ROWS); } - private static IndexBuilder makeRollupBuilder(DimensionsSpec dimensionsSpec, AggregatorFactory[] aggs, boolean autoSchema) + private static IndexBuilder makeRollupBuilder( + DimensionsSpec dimensionsSpec, + AggregatorFactory[] aggs, + boolean autoSchema + ) { File tmp = FileUtils.createTempDir(); CLOSER.register(tmp::delete); return IndexBuilder.create() .tmpDir(tmp) .schema( - IncrementalIndexSchema.builder() - .withDimensionsSpec(dimensionsSpec) - .withMetrics(aggs) - .withRollup(true) - .withMinTimestamp(TIMESTAMP.getMillis()) - .withProjections(autoSchema ? AUTO_ROLLUP_PROJECTIONS : ROLLUP_PROJECTIONS) - .build() + IncrementalIndexSchema + .builder() + .withDimensionsSpec(dimensionsSpec) + .withMetrics(aggs) + .withRollup(true) + .withMinTimestamp(UTC_MIDNIGHT.getMillis()) + .withProjections(autoSchema ? AUTO_ROLLUP_PROJECTIONS : ROLLUP_PROJECTIONS) + .build() ) .writeNullColumns(true) .rows(ROLLUP_ROWS); From 3d1150611079ffe2a6a291c07c12df8a0f0f5a42 Mon Sep 17 00:00:00 2001 From: cecemei Date: Thu, 14 Aug 2025 03:02:18 -0700 Subject: [PATCH 02/18] style --- .../druid/granularity/QueryGranularityTest.java | 6 +++--- .../druid/java/util/common/PeriodGranularityTest.java | 11 +++++------ .../druid/segment/CursorFactoryProjectionTest.java | 5 ++--- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java b/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java index 00a07e7219d0..d32fbb172de0 100644 --- a/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java +++ b/processing/src/test/java/org/apache/druid/granularity/QueryGranularityTest.java @@ -1111,11 +1111,11 @@ public void testFromVirtualColumn() Assert.assertEquals(Granularities.DAY, Granularities.fromVirtualColumn(day)); Assert.assertEquals(Granularities.HOUR, Granularities.fromVirtualColumn(hourlyNonstandardTime)); Assert.assertEquals( - new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.forID("America/Los_Angeles")), + new PeriodGranularity(new Period("PT1H"), null, DateTimes.inferTzFromString("America/Los_Angeles")), Granularities.fromVirtualColumn(hourlyPacificTime) ); Assert.assertEquals( - new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.forID("Asia/Kolkata")), + new PeriodGranularity(new Period("PT1H"), null, DateTimes.inferTzFromString("Asia/Kolkata")), Granularities.fromVirtualColumn(hourlyIndianTime) ); Assert.assertEquals(Granularities.NONE, Granularities.fromVirtualColumn(ceilHour)); @@ -1138,7 +1138,7 @@ public void testFromVirtualColumnExtra() TestExprMacroTable.INSTANCE ); Assert.assertEquals( - new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.forID("America/Los_Angeles")), + new PeriodGranularity(new Period("PT1H"), null, DateTimes.inferTzFromString("America/Los_Angeles")), Granularities.fromVirtualColumn(formatFloor) ); diff --git a/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java b/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java index a51bee9a0c42..4b54c9290bf2 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/PeriodGranularityTest.java @@ -20,7 +20,6 @@ package org.apache.druid.java.util.common; import org.apache.druid.java.util.common.granularity.PeriodGranularity; -import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.Period; import org.junit.Test; @@ -31,8 +30,8 @@ public class PeriodGranularityTest PeriodGranularity UTC_PT1H = new PeriodGranularity(new Period("PT1H"), null, DateTimeZone.UTC); PeriodGranularity UTC_PT1M = new PeriodGranularity(new Period("PT1M"), null, DateTimeZone.UTC); - DateTimeZone PACIFIC_TZ = DateTimeZone.forID("America/Los_Angeles"); - DateTimeZone INDIAN_TZ = DateTimeZone.forID("Asia/Kolkata"); + DateTimeZone PACIFIC_TZ = DateTimes.inferTzFromString("America/Los_Angeles"); + DateTimeZone INDIAN_TZ = DateTimes.inferTzFromString("Asia/Kolkata"); @Test public void testCanBeMappedTo_sameTimeZone() @@ -89,13 +88,13 @@ public void testCanBeMappedTo_withOrigin() { Assertions.assertFalse(new PeriodGranularity( new Period("PT1H"), - new DateTime(), + DateTimes.nowUtc(), DateTimeZone.UTC ).canBeMappedTo(new PeriodGranularity(new Period("PT2H"), null, DateTimeZone.UTC))); Assertions.assertFalse(UTC_PT1H.canBeMappedTo(new PeriodGranularity( new Period("PT1H"), - new DateTime(), + DateTimes.nowUtc(), DateTimeZone.UTC ))); } @@ -109,7 +108,7 @@ public void testCanBeMappedTo_differentTimeZone() Assertions.assertTrue(pacificPT1H.canBeMappedTo(new PeriodGranularity( new Period("PT3H"), null, - DateTimeZone.forID("America/New_York") + DateTimes.inferTzFromString("America/New_York") ))); Assertions.assertTrue(pacificPT1H.canBeMappedTo(new PeriodGranularity( new Period("P1M"), diff --git a/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java b/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java index cd6363313079..6399bb68bc0a 100644 --- a/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java +++ b/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java @@ -86,7 +86,6 @@ import org.apache.druid.segment.virtual.NestedFieldVirtualColumn; import org.apache.druid.testing.InitializedNullHandlingTest; import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; import org.joda.time.Period; import org.junit.AfterClass; import org.junit.Assert; @@ -978,7 +977,7 @@ public void testQueryGranularityFitsProjectionGranularityWithTimeZone() .setGranularity(new PeriodGranularity( new Period("PT1H"), null, - DateTimeZone.forID("America/Los_Angeles") + DateTimes.inferTzFromString("America/Los_Angeles") )); } else { queryBuilder.setGranularity(Granularities.ALL) @@ -1024,7 +1023,7 @@ public void testQueryGranularityDoesNotFitProjectionGranularityWithTimeZone() .setGranularity(new PeriodGranularity( new Period("PT1H"), null, - DateTimeZone.forID("Asia/Kolkata") + DateTimes.inferTzFromString("Asia/Kolkata") )); } else { queryBuilder.setGranularity(Granularities.ALL) From 1e752ea0eaf2d2325fab529b49145926f14d7bff Mon Sep 17 00:00:00 2001 From: cecemei Date: Thu, 14 Aug 2025 13:50:29 -0700 Subject: [PATCH 03/18] more-gran --- .../input/impl/AggregateProjectionSpec.java | 9 +++- .../common/granularity/Granularities.java | 45 +++++-------------- .../expression/TimestampFloorExprMacro.java | 9 ++-- .../segment/AggregateProjectionMetadata.java | 3 ++ .../granularity/QueryGranularityTest.java | 23 ++++++---- 5 files changed, 43 insertions(+), 46 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java b/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java index d9e09cc6fba7..aab86844e58c 100644 --- a/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java +++ b/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java @@ -230,10 +230,15 @@ private static ProjectionOrdering computeOrdering(VirtualColumns virtualColumns, } else { final VirtualColumn vc = virtualColumns.getVirtualColumn(dimension.getName()); final Granularity maybeGranularity = Granularities.fromVirtualColumn(vc); - if (granularity == null && maybeGranularity != null) { + if (maybeGranularity == null || maybeGranularity.equals(Granularities.ALL)) { + // no __time in inputs or not supported, skip + continue; + } + if (granularity == null) { granularity = maybeGranularity; timeColumnName = dimension.getName(); - } else if (granularity != null && maybeGranularity != null && maybeGranularity.isFinerThan(granularity)) { + } else if (maybeGranularity.isFinerThan(granularity)) { + // finer is not a perfect check here, we should rather compute a compatible granularity granularity = maybeGranularity; timeColumnName = dimension.getName(); } diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java index 243f5f955c49..0a3b3effc266 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularities.java @@ -37,8 +37,6 @@ import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; -import org.joda.time.DateTimeZone; -import org.joda.time.Duration; import javax.annotation.Nullable; import java.util.Arrays; @@ -146,10 +144,11 @@ public static CursorBuildSpec decorateCursorBuildSpec(Query query, CursorBuil /** * Translates a {@link Granularity} to a {@link ExpressionVirtualColumn} on {@link ColumnHolder#TIME_COLUMN_NAME} of - * the equivalent grouping column. If granularity is {@link #ALL}, this method returns null since we are not grouping - * on time. If granularity is a {@link PeriodGranularity} with UTC timezone and no origin, this method returns a - * virtual column with {@link TimestampFloorExprMacro.TimestampFloorExpr} of the specified period. If granularity is - * {@link #NONE}, or any other kind of granularity (duration, period with non-utc timezone or origin) this method + * the equivalent grouping column. + *