From feb7425b69ffd41e32645f4b108624a54d052017 Mon Sep 17 00:00:00 2001 From: Slim Bouguerra Date: Wed, 11 Oct 2017 19:28:13 -0700 Subject: [PATCH 1/8] Introduce System wide property to select how to store double. Set the default to store as float Change-Id: Id85cca04ed0e7ecbce78624168c586dcc2adafaa --- .../aggregation/SimpleDoubleAggregatorFactory.java | 10 ++++++++++ .../first/DoubleFirstAggregatorFactory.java | 6 ++++++ .../aggregation/last/DoubleLastAggregatorFactory.java | 7 +++++++ 3 files changed, 23 insertions(+) diff --git a/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java index 24441d029028..1a6ec2829ae0 100644 --- a/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java @@ -38,6 +38,7 @@ public abstract class SimpleDoubleAggregatorFactory extends AggregatorFactory protected final String fieldName; protected final String expression; protected final ExprMacroTable macroTable; + protected final boolean storeDoubleAsFloat; public SimpleDoubleAggregatorFactory( ExprMacroTable macroTable, @@ -50,6 +51,7 @@ public SimpleDoubleAggregatorFactory( this.fieldName = fieldName; this.name = name; this.expression = expression; + this.storeDoubleAsFloat = storeDoubleAsFloat(); Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); Preconditions.checkArgument( fieldName == null ^ expression == null, @@ -75,6 +77,9 @@ public Object deserialize(Object object) @Override public String getTypeName() { + if (storeDoubleAsFloat) { + return "float"; + } return "double"; } @@ -138,4 +143,9 @@ public String getExpression() { return expression; } + + public static boolean storeDoubleAsFloat() + { + return Boolean.valueOf(System.getProperty("druid.indexing.store.double.as.float", "true")); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java index 1fb8564484d8..631a877679f4 100644 --- a/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java @@ -33,6 +33,7 @@ import io.druid.query.aggregation.AggregatorUtil; import io.druid.query.aggregation.BufferAggregator; import io.druid.query.aggregation.AggregateCombiner; +import io.druid.query.aggregation.SimpleDoubleAggregatorFactory; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.ObjectColumnSelector; @@ -59,6 +60,7 @@ public class DoubleFirstAggregatorFactory extends AggregatorFactory private final String fieldName; private final String name; + private final boolean storeDoubleAsFloat; @JsonCreator public DoubleFirstAggregatorFactory( @@ -71,6 +73,7 @@ public DoubleFirstAggregatorFactory( this.name = name; this.fieldName = fieldName; + this.storeDoubleAsFloat = SimpleDoubleAggregatorFactory.storeDoubleAsFloat(); } @Override @@ -222,6 +225,9 @@ public byte[] getCacheKey() @Override public String getTypeName() { + if (storeDoubleAsFloat) { + return "float"; + } return "double"; } diff --git a/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java index c7a9a276e355..791e02148925 100644 --- a/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java @@ -32,6 +32,7 @@ import io.druid.query.aggregation.AggregatorFactoryNotMergeableException; import io.druid.query.aggregation.AggregatorUtil; import io.druid.query.aggregation.BufferAggregator; +import io.druid.query.aggregation.SimpleDoubleAggregatorFactory; import io.druid.query.aggregation.first.DoubleFirstAggregatorFactory; import io.druid.query.aggregation.first.LongFirstAggregatorFactory; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; @@ -51,6 +52,7 @@ public class DoubleLastAggregatorFactory extends AggregatorFactory private final String fieldName; private final String name; + private final boolean storeDoubleAsFloat; @JsonCreator public DoubleLastAggregatorFactory( @@ -62,6 +64,7 @@ public DoubleLastAggregatorFactory( Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; + this.storeDoubleAsFloat = SimpleDoubleAggregatorFactory.storeDoubleAsFloat(); } @Override @@ -213,6 +216,10 @@ public byte[] getCacheKey() @Override public String getTypeName() { + + if (storeDoubleAsFloat) { + return "float"; + } return "double"; } From aa833931e68cb1bab47fedfceaf6c5c724778980 Mon Sep 17 00:00:00 2001 From: Slim Bouguerra Date: Thu, 12 Oct 2017 18:50:49 -0700 Subject: [PATCH 2/8] fix tests Change-Id: Ib42db724b8a8f032d204b58c366caaeabdd0d939 --- pom.xml | 1 + .../GroupByTimeseriesQueryRunnerTest.java | 4 ++-- .../druid/query/scan/ScanQueryRunnerTest.java | 22 ++++++++++++++++--- .../timeseries/TimeseriesQueryRunnerTest.java | 4 ++-- .../java/io/druid/segment/TestHelper.java | 2 +- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index f86bcc1858fa..4902a038310c 100644 --- a/pom.xml +++ b/pom.xml @@ -1036,6 +1036,7 @@ -Xmx3000m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 -Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager + -Ddruid.indexing.store.double.as.float=false true diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByTimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByTimeseriesQueryRunnerTest.java index bb7edc210694..96e1bd0eef4c 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByTimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByTimeseriesQueryRunnerTest.java @@ -172,8 +172,8 @@ public void testFullOnTimeseriesMaxMin() final TimeseriesResultValue value = result.getValue(); - Assert.assertEquals(result.toString(), 1870.061029, value.getDoubleMetric("maxIndex"), 0.0); - Assert.assertEquals(result.toString(), 59.021022, value.getDoubleMetric("minIndex"), 0.0); + Assert.assertEquals(result.toString(), 1870.061029, value.getDoubleMetric("maxIndex"), 1870.061029 * 1e-6); + Assert.assertEquals(result.toString(), 59.021022, value.getDoubleMetric("minIndex"), 59.021022 * 1e-6); } diff --git a/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java b/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java index 84d5a3e5fb01..d5a454d2b20c 100644 --- a/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java @@ -719,8 +719,13 @@ private static void verify( if (actVal instanceof String[]) { actVal = Arrays.asList((String[]) actVal); } - - Assert.assertEquals("invalid value for " + ex.getKey(), ex.getValue(), actVal); + Object exValue = ex.getValue(); + if (exValue instanceof Double || exValue instanceof Float) { + final double expectedDoubleValue = ((Number) exValue).doubleValue(); + Assert.assertEquals("invalid value for " + ex.getKey(), expectedDoubleValue, ((Number) actVal).doubleValue(), expectedDoubleValue * 1e-6); + } else { + Assert.assertEquals("invalid value for " + ex.getKey(), ex.getValue(), actVal); + } } for (Map.Entry ac : acHolder.entrySet()) { @@ -731,7 +736,18 @@ private static void verify( actVal = Arrays.asList((String[]) actVal); } - Assert.assertEquals("invalid value for " + ac.getKey(), exVal, actVal); + if (exVal instanceof Double || exVal instanceof Float) { + final double exDoubleValue = ((Number) exVal).doubleValue(); + Assert.assertEquals( + "invalid value for " + ac.getKey(), + exDoubleValue, + ((Number) actVal).doubleValue(), + exDoubleValue * 1e-6 + ); + } else { + Assert.assertEquals("invalid value for " + ac.getKey(), exVal, actVal); + } + } } diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java index 737acd55ab92..f4fafbcec573 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -308,8 +308,8 @@ public void testFullOnTimeseriesMaxMin() final TimeseriesResultValue value = result.getValue(); - Assert.assertEquals(result.toString(), 1870.061029, value.getDoubleMetric("maxIndex"), 0.0); - Assert.assertEquals(result.toString(), 59.021022, value.getDoubleMetric("minIndex"), 0.0); + Assert.assertEquals(result.toString(), 1870.061029, value.getDoubleMetric("maxIndex"), 1870.061029 * 1e-6); + Assert.assertEquals(result.toString(), 59.021022, value.getDoubleMetric("minIndex"), 59.021022 * 1e-6); } @Test diff --git a/processing/src/test/java/io/druid/segment/TestHelper.java b/processing/src/test/java/io/druid/segment/TestHelper.java index a0f5e62d5d21..5ca389305140 100644 --- a/processing/src/test/java/io/druid/segment/TestHelper.java +++ b/processing/src/test/java/io/druid/segment/TestHelper.java @@ -323,7 +323,7 @@ private static void assertRow(String msg, Row expected, Row actual) StringUtils.format("%s: key[%s]", msg, key), ((Number) expectedValue).doubleValue(), ((Number) actualValue).doubleValue(), - ((Number) expectedValue).doubleValue() * 1e-6 + Math.abs(((Number) expectedValue).doubleValue() * 1e-6) ); } else { Assert.assertEquals( From d6e03dedc3f0cf650b276cf25c29acdf424f78a0 Mon Sep 17 00:00:00 2001 From: Slim Bouguerra Date: Fri, 13 Oct 2017 15:12:32 -0700 Subject: [PATCH 3/8] Change the property name Change-Id: I3ed69f79fc56e3735bc8f3a097f52a9f932b4734 --- pom.xml | 2 +- .../query/aggregation/SimpleDoubleAggregatorFactory.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 4902a038310c..e8b8dfe94b30 100644 --- a/pom.xml +++ b/pom.xml @@ -1036,7 +1036,7 @@ -Xmx3000m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 -Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager - -Ddruid.indexing.store.double.as.float=false + -Ddruid.indexing.doubleStorage=double true diff --git a/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java index 1a6ec2829ae0..2c12274a9438 100644 --- a/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import io.druid.java.util.common.StringUtils; import io.druid.math.expr.ExprMacroTable; import io.druid.math.expr.Parser; import io.druid.segment.ColumnSelectorFactory; @@ -34,6 +35,7 @@ public abstract class SimpleDoubleAggregatorFactory extends AggregatorFactory { + public final static String DOUBLE_STORAGE_TYPE_PROPERTY = "druid.indexing.doubleStorage"; protected final String name; protected final String fieldName; protected final String expression; @@ -146,6 +148,7 @@ public String getExpression() public static boolean storeDoubleAsFloat() { - return Boolean.valueOf(System.getProperty("druid.indexing.store.double.as.float", "true")); + String value = System.getProperty(DOUBLE_STORAGE_TYPE_PROPERTY, "float"); + return !StringUtils.toLowerCase(value).equals("double"); } } From 54bff055f6ea7ca1a61008585767d799a660ff19 Mon Sep 17 00:00:00 2001 From: Slim Bouguerra Date: Fri, 13 Oct 2017 17:09:00 -0700 Subject: [PATCH 4/8] add tests and make default distribution store doubles as 64bits Change-Id: I237b07829117ac61e247a6124423b03992f550f2 --- .../druid/_common/common.runtime.properties | 5 + .../druid/_common/common.runtime.properties | 5 + .../io/druid/query/DoubleStorageTest.java | 369 ++++++++++++++++++ .../druid/query/scan/ScanQueryRunnerTest.java | 2 +- 4 files changed, 380 insertions(+), 1 deletion(-) create mode 100644 processing/src/test/java/io/druid/query/DoubleStorageTest.java diff --git a/examples/conf-quickstart/druid/_common/common.runtime.properties b/examples/conf-quickstart/druid/_common/common.runtime.properties index ba73feb685fc..93e3213fefe7 100644 --- a/examples/conf-quickstart/druid/_common/common.runtime.properties +++ b/examples/conf-quickstart/druid/_common/common.runtime.properties @@ -116,3 +116,8 @@ druid.selectors.coordinator.serviceName=druid/coordinator druid.monitoring.monitors=["com.metamx.metrics.JvmMonitor"] druid.emitter=logging druid.emitter.logging.logLevel=info + +# Storage type of double columns +# ommiting this will lead to index double as float at the storage layer + +druid.indexing.doubleStorage=double \ No newline at end of file diff --git a/examples/conf/druid/_common/common.runtime.properties b/examples/conf/druid/_common/common.runtime.properties index 641ef03c6151..727acce1ae8e 100644 --- a/examples/conf/druid/_common/common.runtime.properties +++ b/examples/conf/druid/_common/common.runtime.properties @@ -115,3 +115,8 @@ druid.selectors.coordinator.serviceName=druid/coordinator druid.monitoring.monitors=["com.metamx.metrics.JvmMonitor"] druid.emitter=logging druid.emitter.logging.logLevel=info + +# Storage type of double columns +# ommiting this will lead to index double as float at the storage layer + +druid.indexing.doubleStorage=double \ No newline at end of file diff --git a/processing/src/test/java/io/druid/query/DoubleStorageTest.java b/processing/src/test/java/io/druid/query/DoubleStorageTest.java new file mode 100644 index 000000000000..07911c87883e --- /dev/null +++ b/processing/src/test/java/io/druid/query/DoubleStorageTest.java @@ -0,0 +1,369 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query; + +import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import io.druid.data.input.impl.DimensionsSpec; +import io.druid.data.input.impl.InputRowParser; +import io.druid.data.input.impl.JSONParseSpec; +import io.druid.data.input.impl.MapInputRowParser; +import io.druid.data.input.impl.SpatialDimensionSchema; +import io.druid.data.input.impl.TimestampSpec; +import io.druid.java.util.common.DateTimes; +import io.druid.java.util.common.Intervals; +import io.druid.java.util.common.guava.Sequences; +import io.druid.query.aggregation.DoubleSumAggregatorFactory; +import io.druid.query.metadata.SegmentMetadataQueryConfig; +import io.druid.query.metadata.SegmentMetadataQueryQueryToolChest; +import io.druid.query.metadata.SegmentMetadataQueryRunnerFactory; +import io.druid.query.metadata.metadata.ColumnAnalysis; +import io.druid.query.metadata.metadata.ListColumnIncluderator; +import io.druid.query.metadata.metadata.SegmentAnalysis; +import io.druid.query.metadata.metadata.SegmentMetadataQuery; + +import io.druid.query.scan.ScanQuery; +import io.druid.query.scan.ScanQueryConfig; +import io.druid.query.scan.ScanQueryEngine; +import io.druid.query.scan.ScanQueryQueryToolChest; +import io.druid.query.scan.ScanQueryRunnerFactory; +import io.druid.query.scan.ScanResultValue; +import io.druid.query.spec.LegacySegmentSpec; +import io.druid.segment.IndexIO; +import io.druid.segment.IndexMergerV9; +import io.druid.segment.IndexSpec; +import io.druid.segment.QueryableIndex; +import io.druid.segment.QueryableIndexSegment; +import io.druid.segment.TestHelper; +import io.druid.segment.column.ValueType; +import io.druid.segment.incremental.IncrementalIndex; +import io.druid.segment.incremental.IncrementalIndexSchema; +import io.druid.segment.incremental.IndexSizeExceededException; +import org.joda.time.Interval; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.File; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import static io.druid.query.aggregation.SimpleDoubleAggregatorFactory.DOUBLE_STORAGE_TYPE_PROPERTY; +import static io.druid.query.scan.ScanQueryRunnerTest.verify; + +@RunWith(Parameterized.class) +public class DoubleStorageTest +{ + + private static final SegmentMetadataQueryRunnerFactory METADATA_QR_FACTORY = new SegmentMetadataQueryRunnerFactory( + new SegmentMetadataQueryQueryToolChest(new SegmentMetadataQueryConfig()), + QueryRunnerTestHelper.NOOP_QUERYWATCHER + ); + + private static final ScanQueryQueryToolChest scanQueryQueryToolChest = new ScanQueryQueryToolChest( + new ScanQueryConfig(), + DefaultGenericQueryMetricsFactory.instance() + ); + + private static final ScanQueryRunnerFactory SCAN_QUERY_RUNNER_FACTORY = new ScanQueryRunnerFactory( + scanQueryQueryToolChest, + new ScanQueryEngine() + ); + + private ScanQuery.ScanQueryBuilder newTestQuery() + { + return ScanQuery.newScanQueryBuilder() + .dataSource(new TableDataSource(QueryRunnerTestHelper.dataSource)) + .columns(Arrays.asList()) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .limit(Integer.MAX_VALUE) + .legacy(false); + } + + + private static final IndexMergerV9 INDEX_MERGER_V9 = TestHelper.getTestIndexMergerV9(); + private static final IndexIO INDEX_IO = TestHelper.getTestIndexIO(); + private static final Integer MAX_ROWS = 10; + private static final String TIME_COLUMN = "__time"; + private static final String DIM_NAME = "testDimName"; + private static final String DIM_VALUE = "testDimValue"; + private static final String DIM_FLOAT_NAME = "testDimFloatName"; + private static final String SEGMENT_ID = "segmentId"; + private static final Interval INTERVAL = Intervals.of("2011-01-13T00:00:00.000Z/2011-01-22T00:00:00.001Z"); + + private static final InputRowParser> ROW_PARSER = new MapInputRowParser( + new JSONParseSpec( + new TimestampSpec(TIME_COLUMN, "auto", null), + new DimensionsSpec( + DimensionsSpec.getDefaultSchemas(ImmutableList.of(DIM_NAME)), + ImmutableList.of(DIM_FLOAT_NAME), + ImmutableList.of() + ), + null, + null + ) + ); + + private QueryableIndex index; + private final SegmentAnalysis expectedSegmentAnalysis; + private final String storeDoubleAs; + + public DoubleStorageTest( + String storeDoubleAs, + SegmentAnalysis expectedSegmentAnalysis + ) throws IOException + { + this.storeDoubleAs = storeDoubleAs; + this.expectedSegmentAnalysis = expectedSegmentAnalysis; + } + + @Parameterized.Parameters + public static Collection dataFeeder() throws IOException + { + SegmentAnalysis expectedSegmentAnalysisDouble = new SegmentAnalysis( + "segmentId", + ImmutableList.of(INTERVAL), + ImmutableMap.of( + TIME_COLUMN, + new ColumnAnalysis( + ValueType.LONG.toString(), + false, + 100, + null, + null, + null, + null + ), + DIM_NAME, + new ColumnAnalysis( + ValueType.STRING.toString(), + false, + 120, + 1, + DIM_VALUE, + DIM_VALUE, + null + ), + DIM_FLOAT_NAME, + new ColumnAnalysis( + ValueType.DOUBLE.toString(), + false, + 80, + null, + null, + null, + null + ) + ), 330, + MAX_ROWS, + null, + null, + null, + null + ); + + SegmentAnalysis expectedSegmentAnalysisFloat = new SegmentAnalysis( + "segmentId", + ImmutableList.of(INTERVAL), + ImmutableMap.of( + TIME_COLUMN, + new ColumnAnalysis( + ValueType.LONG.toString(), + false, + 100, + null, + null, + null, + null + ), + DIM_NAME, + new ColumnAnalysis( + ValueType.STRING.toString(), + false, + 120, + 1, + DIM_VALUE, + DIM_VALUE, + null + ), + DIM_FLOAT_NAME, + new ColumnAnalysis( + ValueType.FLOAT.toString(), + false, + 80, + null, + null, + null, + null + ) + ), 330, + MAX_ROWS, + null, + null, + null, + null + ); + + return ImmutableList.of( + new Object[]{"double", expectedSegmentAnalysisDouble}, + new Object[]{"float", expectedSegmentAnalysisFloat} + ); + } + + @Before + public void setup() throws IOException + { + index = buildIndex(storeDoubleAs); + } + + @Test + public void testMetaDataAnalysis() throws IndexSizeExceededException + { + QueryRunner runner = QueryRunnerTestHelper.makeQueryRunner( + METADATA_QR_FACTORY, + SEGMENT_ID, + new QueryableIndexSegment("segmentId", index), + null + ); + + + SegmentMetadataQuery segmentMetadataQuery = Druids.newSegmentMetadataQueryBuilder() + .dataSource("testing") + .intervals(ImmutableList.of(INTERVAL)) + .toInclude(new ListColumnIncluderator(Arrays.asList( + TIME_COLUMN, + DIM_NAME, + DIM_FLOAT_NAME + ))) + .analysisTypes( + SegmentMetadataQuery.AnalysisType.CARDINALITY, + SegmentMetadataQuery.AnalysisType.SIZE, + SegmentMetadataQuery.AnalysisType.INTERVAL, + SegmentMetadataQuery.AnalysisType.MINMAX + ) + .merge(true) + .build(); + List results = Sequences.toList( + runner.run(QueryPlus.wrap(segmentMetadataQuery), Maps.newHashMap()), + Lists.newArrayList() + ); + + Assert.assertEquals(Arrays.asList(expectedSegmentAnalysis), results); + + } + + @Test + public void testSelectValues() + { + QueryRunner runner = QueryRunnerTestHelper.makeQueryRunner( + SCAN_QUERY_RUNNER_FACTORY, + SEGMENT_ID, + new QueryableIndexSegment("segmentId", index), + null + ); + + ScanQuery query = newTestQuery() + .intervals(new LegacySegmentSpec(INTERVAL)) + .virtualColumns() + .build(); + + HashMap context = new HashMap(); + Iterable results = Sequences.toList( + runner.run(QueryPlus.wrap(query), context), + Lists.newArrayList() + ); + + ScanResultValue expectedScanResult = new ScanResultValue( + SEGMENT_ID, + ImmutableList.of(TIME_COLUMN, DIM_NAME, DIM_FLOAT_NAME), + getStreamOfEvents().collect(Collectors.toList()) + ); + List expectedResults = Lists.newArrayList(expectedScanResult); + verify(expectedResults, results); + } + + private static QueryableIndex buildIndex(String storeDoubleAsFloat) throws IOException + { + String oldValue = System.getProperty(DOUBLE_STORAGE_TYPE_PROPERTY); + System.setProperty(DOUBLE_STORAGE_TYPE_PROPERTY, storeDoubleAsFloat); + final IncrementalIndexSchema schema = new IncrementalIndexSchema.Builder() + .withMinTimestamp(DateTimes.of("2011-01-13T00:00:00.000Z").getMillis()) + .withDimensionsSpec(ROW_PARSER) + .withMetrics( + new DoubleSumAggregatorFactory(DIM_FLOAT_NAME, DIM_FLOAT_NAME) + ) + .build(); + + final IncrementalIndex index = new IncrementalIndex.Builder() + .setIndexSchema(schema) + .setMaxRowCount(MAX_ROWS) + .buildOnheap(); + + + getStreamOfEvents().forEach(o -> { + try { + index.add(ROW_PARSER.parse((Map) o)); + } + catch (IndexSizeExceededException e) { + Throwables.propagate(e); + } + }); + + if (oldValue == null) { + System.clearProperty(DOUBLE_STORAGE_TYPE_PROPERTY); + } else { + System.setProperty(DOUBLE_STORAGE_TYPE_PROPERTY, oldValue); + } + File someTmpFile = File.createTempFile("billy", "yay"); + someTmpFile.delete(); + someTmpFile.mkdirs(); + INDEX_MERGER_V9.persist(index, someTmpFile, new IndexSpec()); + someTmpFile.delete(); + return INDEX_IO.loadIndex(someTmpFile); + } + + @After + public void cleanUp() throws IOException + { + index.close(); + } + + private static Stream getStreamOfEvents() + { + return IntStream.range(0, MAX_ROWS).mapToObj(i -> ImmutableMap.of( + TIME_COLUMN, DateTimes.of("2011-01-13T00:00:00.000Z").plusDays(i).getMillis(), + DIM_NAME, DIM_VALUE, + DIM_FLOAT_NAME, i / 1.6179775280898876 + )); + } + +} diff --git a/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java b/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java index d5a454d2b20c..c3a77e33f66d 100644 --- a/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/scan/ScanQueryRunnerTest.java @@ -688,7 +688,7 @@ private List toExpected( return expected; } - private static void verify( + public static void verify( Iterable expectedResults, Iterable actualResults ) From 9f4107a6b6f14e1b3b50c239c4c4c6023b43060a Mon Sep 17 00:00:00 2001 From: Slim Bouguerra Date: Fri, 13 Oct 2017 20:21:33 -0700 Subject: [PATCH 5/8] adding mvn argument to parallel-test profile Change-Id: Iae5d1328f901c4876b133894fa37e0d9a4162b05 --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index e8b8dfe94b30..85ce8750fd69 100644 --- a/pom.xml +++ b/pom.xml @@ -1266,6 +1266,7 @@ -Xmx768m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 -Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager + -Ddruid.indexing.doubleStorage=double true From e4690b2b0be085e3675c35a0c1b1aebb188698fb Mon Sep 17 00:00:00 2001 From: Slim Date: Mon, 16 Oct 2017 12:02:09 -0700 Subject: [PATCH 6/8] move property name and helper function to io.druid.segment.column.Column Change-Id: I62ea903d332515de2b7ca45c02587a1b015cb065 --- .../aggregation/SimpleDoubleAggregatorFactory.java | 10 ++-------- .../first/DoubleFirstAggregatorFactory.java | 3 +-- .../aggregation/last/DoubleLastAggregatorFactory.java | 3 +-- .../src/main/java/io/druid/segment/column/Column.java | 10 ++++++++++ .../test/java/io/druid/query/DoubleStorageTest.java | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java index ba82663c8553..e627c126c642 100644 --- a/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/SimpleDoubleAggregatorFactory.java @@ -22,11 +22,11 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import io.druid.java.util.common.StringUtils; import io.druid.math.expr.ExprMacroTable; import io.druid.math.expr.Parser; import io.druid.segment.BaseDoubleColumnValueSelector; import io.druid.segment.ColumnSelectorFactory; +import io.druid.segment.column.Column; import java.util.Collections; import java.util.Comparator; @@ -35,7 +35,6 @@ public abstract class SimpleDoubleAggregatorFactory extends AggregatorFactory { - public final static String DOUBLE_STORAGE_TYPE_PROPERTY = "druid.indexing.doubleStorage"; protected final String name; protected final String fieldName; protected final String expression; @@ -53,7 +52,7 @@ public SimpleDoubleAggregatorFactory( this.fieldName = fieldName; this.name = name; this.expression = expression; - this.storeDoubleAsFloat = storeDoubleAsFloat(); + this.storeDoubleAsFloat = Column.storeDoubleAsFloat(); Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); Preconditions.checkArgument( fieldName == null ^ expression == null, @@ -152,9 +151,4 @@ public String getExpression() return expression; } - public static boolean storeDoubleAsFloat() - { - String value = System.getProperty(DOUBLE_STORAGE_TYPE_PROPERTY, "float"); - return !StringUtils.toLowerCase(value).equals("double"); - } } diff --git a/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java index f42b55db5e16..561238fb3381 100644 --- a/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java @@ -33,7 +33,6 @@ import io.druid.query.aggregation.AggregatorFactoryNotMergeableException; import io.druid.query.aggregation.AggregatorUtil; import io.druid.query.aggregation.BufferAggregator; -import io.druid.query.aggregation.SimpleDoubleAggregatorFactory; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.BaseObjectColumnValueSelector; import io.druid.segment.ColumnSelectorFactory; @@ -73,7 +72,7 @@ public DoubleFirstAggregatorFactory( this.name = name; this.fieldName = fieldName; - this.storeDoubleAsFloat = SimpleDoubleAggregatorFactory.storeDoubleAsFloat(); + this.storeDoubleAsFloat = Column.storeDoubleAsFloat(); } @Override diff --git a/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java index 4f397484c481..a0ed875ecda5 100644 --- a/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/last/DoubleLastAggregatorFactory.java @@ -32,7 +32,6 @@ import io.druid.query.aggregation.AggregatorFactoryNotMergeableException; import io.druid.query.aggregation.AggregatorUtil; import io.druid.query.aggregation.BufferAggregator; -import io.druid.query.aggregation.SimpleDoubleAggregatorFactory; import io.druid.query.aggregation.first.DoubleFirstAggregatorFactory; import io.druid.query.aggregation.first.LongFirstAggregatorFactory; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; @@ -64,7 +63,7 @@ public DoubleLastAggregatorFactory( Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; - this.storeDoubleAsFloat = SimpleDoubleAggregatorFactory.storeDoubleAsFloat(); + this.storeDoubleAsFloat = Column.storeDoubleAsFloat(); } @Override diff --git a/processing/src/main/java/io/druid/segment/column/Column.java b/processing/src/main/java/io/druid/segment/column/Column.java index b6b52582ae41..2505170f774b 100644 --- a/processing/src/main/java/io/druid/segment/column/Column.java +++ b/processing/src/main/java/io/druid/segment/column/Column.java @@ -19,11 +19,21 @@ package io.druid.segment.column; +import io.druid.java.util.common.StringUtils; + /** */ public interface Column { String TIME_COLUMN_NAME = "__time"; + String DOUBLE_STORAGE_TYPE_PROPERTY = "druid.indexing.doubleStorage"; + + static boolean storeDoubleAsFloat() + { + String value = System.getProperty(DOUBLE_STORAGE_TYPE_PROPERTY, "float"); + return !StringUtils.toLowerCase(value).equals("double"); + } + ColumnCapabilities getCapabilities(); int getLength(); diff --git a/processing/src/test/java/io/druid/query/DoubleStorageTest.java b/processing/src/test/java/io/druid/query/DoubleStorageTest.java index 07911c87883e..dbdeb81f7890 100644 --- a/processing/src/test/java/io/druid/query/DoubleStorageTest.java +++ b/processing/src/test/java/io/druid/query/DoubleStorageTest.java @@ -77,7 +77,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; -import static io.druid.query.aggregation.SimpleDoubleAggregatorFactory.DOUBLE_STORAGE_TYPE_PROPERTY; +import static io.druid.segment.column.Column.DOUBLE_STORAGE_TYPE_PROPERTY; import static io.druid.query.scan.ScanQueryRunnerTest.verify; @RunWith(Parameterized.class) From f951820a9c03822490baf1f16a453db1fb8af1eb Mon Sep 17 00:00:00 2001 From: Slim Date: Mon, 16 Oct 2017 14:26:16 -0700 Subject: [PATCH 7/8] fix docs and clean style Change-Id: I726abb8f52d25dc9dc62ad98814c5feda5e4d065 --- docs/content/configuration/index.md | 9 +++++++++ .../druid/_common/common.runtime.properties | 2 +- examples/conf/druid/_common/common.runtime.properties | 2 +- pom.xml | 6 ++++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/docs/content/configuration/index.md b/docs/content/configuration/index.md index 15816b3ad7a0..505dbffbbdda 100644 --- a/docs/content/configuration/index.md +++ b/docs/content/configuration/index.md @@ -395,3 +395,12 @@ the following properties.
JavaScript-based functionality is disabled by default. Please refer to the Druid JavaScript programming guide for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
+ +### Double Column storage + +Originaly druid storage layer uses a 32 bits float representation to store double columns at indexing time. +To use proper 64 bits to store double columns please use the system wide property `druid.indexing.doubleStorage=double`. + +|Property|Description|Default| +|--------|-----------|-------| +|`druid.indexing.doubleStorage`|Set to "double" to use 64 bits double representation for double columns.|float| diff --git a/examples/conf-quickstart/druid/_common/common.runtime.properties b/examples/conf-quickstart/druid/_common/common.runtime.properties index 93e3213fefe7..fd131b878cce 100644 --- a/examples/conf-quickstart/druid/_common/common.runtime.properties +++ b/examples/conf-quickstart/druid/_common/common.runtime.properties @@ -120,4 +120,4 @@ druid.emitter.logging.logLevel=info # Storage type of double columns # ommiting this will lead to index double as float at the storage layer -druid.indexing.doubleStorage=double \ No newline at end of file +druid.indexing.doubleStorage=double diff --git a/examples/conf/druid/_common/common.runtime.properties b/examples/conf/druid/_common/common.runtime.properties index 727acce1ae8e..a018fa01780c 100644 --- a/examples/conf/druid/_common/common.runtime.properties +++ b/examples/conf/druid/_common/common.runtime.properties @@ -119,4 +119,4 @@ druid.emitter.logging.logLevel=info # Storage type of double columns # ommiting this will lead to index double as float at the storage layer -druid.indexing.doubleStorage=double \ No newline at end of file +druid.indexing.doubleStorage=double diff --git a/pom.xml b/pom.xml index 85ce8750fd69..6029a1393305 100644 --- a/pom.xml +++ b/pom.xml @@ -1036,6 +1036,7 @@ -Xmx3000m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 -Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager + -Ddruid.indexing.doubleStorage=double @@ -1265,8 +1266,9 @@ -Xmx768m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 - -Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager - -Ddruid.indexing.doubleStorage=double + -Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager + + -Ddruid.indexing.doubleStorage=double true From c0a47034febe02e493df9ca8b3b5c18722775695 Mon Sep 17 00:00:00 2001 From: Slim Date: Mon, 16 Oct 2017 15:28:45 -0700 Subject: [PATCH 8/8] fix docs Change-Id: If10f4cf1e51a58285a301af4107ea17fe5e09b6d --- docs/content/configuration/index.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/content/configuration/index.md b/docs/content/configuration/index.md index 505dbffbbdda..9bd7b97ce49c 100644 --- a/docs/content/configuration/index.md +++ b/docs/content/configuration/index.md @@ -398,9 +398,11 @@ JavaScript-based functionality is disabled by default. Please refer to the Druid ### Double Column storage -Originaly druid storage layer uses a 32 bits float representation to store double columns at indexing time. -To use proper 64 bits to store double columns please use the system wide property `druid.indexing.doubleStorage=double`. +Druid's storage layer uses a 32-bit float representation to store columns created by the +doubleSum, doubleMin, and doubleMax aggregators at indexing time. To instead use 64-bit floats +for these columns, please set the system-wide property `druid.indexing.doubleStorage=double`. +This will become the default behavior in a future version of Druid. |Property|Description|Default| |--------|-----------|-------| -|`druid.indexing.doubleStorage`|Set to "double" to use 64 bits double representation for double columns.|float| +|`druid.indexing.doubleStorage`|Set to "double" to use 64-bit double representation for double columns.|float|