From d3aa2f35538043a38a611f9fc246fd72a7d0a5d7 Mon Sep 17 00:00:00 2001 From: Abhishek Radhakrishnan Date: Wed, 8 Apr 2020 18:56:06 -0700 Subject: [PATCH] Preserve the null values for numeric type dimensions post-compaction. (#9622) * Add selector null check to preserve null values as-is. * Fix typo. * add wrapping dimension selector test. * Address review comments. * nit: replace exception type. * uh, float is indeed NOT a special case. --- .../DoubleWrappingDimensionSelector.java | 4 +- .../FloatWrappingDimensionSelector.java | 4 +- .../LongWrappingDimensionSelector.java | 4 +- .../TestNullableDoubleColumnSelector.java | 61 +++++++++ .../TestNullableFloatColumnSelector.java | 62 ++++++++++ .../TestNullableLongColumnSelector.java | 61 +++++++++ .../WrappingDimensionSelectorTest.java | 117 ++++++++++++++++++ .../loading/LocalDataSegmentPuller.java | 2 +- 8 files changed, 311 insertions(+), 4 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java create mode 100644 processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java create mode 100644 processing/src/test/java/org/apache/druid/segment/TestNullableLongColumnSelector.java create mode 100644 processing/src/test/java/org/apache/druid/segment/WrappingDimensionSelectorTest.java diff --git a/processing/src/main/java/org/apache/druid/segment/DoubleWrappingDimensionSelector.java b/processing/src/main/java/org/apache/druid/segment/DoubleWrappingDimensionSelector.java index 89cc99c89b15..4f1dac049f69 100644 --- a/processing/src/main/java/org/apache/druid/segment/DoubleWrappingDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/DoubleWrappingDimensionSelector.java @@ -43,7 +43,9 @@ public DoubleWrappingDimensionSelector( @Override protected String getValue() { - if (extractionFn == null) { + if (selector.isNull()) { + return null; + } else if (extractionFn == null) { return String.valueOf(selector.getDouble()); } else { return extractionFn.apply(selector.getDouble()); diff --git a/processing/src/main/java/org/apache/druid/segment/FloatWrappingDimensionSelector.java b/processing/src/main/java/org/apache/druid/segment/FloatWrappingDimensionSelector.java index 3e39e995fe7c..ec5f958dab9e 100644 --- a/processing/src/main/java/org/apache/druid/segment/FloatWrappingDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/FloatWrappingDimensionSelector.java @@ -40,7 +40,9 @@ public FloatWrappingDimensionSelector(BaseFloatColumnValueSelector selector, @Nu @Override protected String getValue() { - if (extractionFn == null) { + if (selector.isNull()) { + return null; + } else if (extractionFn == null) { return String.valueOf(selector.getFloat()); } else { return extractionFn.apply(selector.getFloat()); diff --git a/processing/src/main/java/org/apache/druid/segment/LongWrappingDimensionSelector.java b/processing/src/main/java/org/apache/druid/segment/LongWrappingDimensionSelector.java index 1388dcf04802..de25d2ebd874 100644 --- a/processing/src/main/java/org/apache/druid/segment/LongWrappingDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/LongWrappingDimensionSelector.java @@ -40,7 +40,9 @@ public LongWrappingDimensionSelector(BaseLongColumnValueSelector selector, @Null @Override protected String getValue() { - if (extractionFn == null) { + if (selector.isNull()) { + return null; + } else if (extractionFn == null) { return String.valueOf(selector.getLong()); } else { return extractionFn.apply(selector.getLong()); diff --git a/processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java b/processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java new file mode 100644 index 000000000000..609ce1e01dce --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java @@ -0,0 +1,61 @@ +/* + * 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.segment; + +import org.apache.druid.common.config.NullHandling; + +public class TestNullableDoubleColumnSelector extends TestDoubleColumnSelector +{ + private final Double[] doubles; + + static { + NullHandling.initializeForTests(); + } + + private int index = 0; + + public TestNullableDoubleColumnSelector(Double[] doubles) + { + this.doubles = doubles; + } + + @Override + public double getDouble() + { + if (doubles[index] != null) { + return doubles[index]; + } else if (NullHandling.replaceWithDefault()) { + return NullHandling.ZERO_DOUBLE; + } else { + throw new IllegalStateException("Should never be invoked when current value is null && SQL-compatible null handling is enabled!"); + } + } + + @Override + public boolean isNull() + { + return !NullHandling.replaceWithDefault() && doubles[index] == null; + } + + public void increment() + { + ++index; + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java b/processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java new file mode 100644 index 000000000000..19b49610de48 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java @@ -0,0 +1,62 @@ +/* + * 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.segment; + +import org.apache.druid.common.config.NullHandling; + +public class TestNullableFloatColumnSelector extends TestFloatColumnSelector +{ + + private final Float[] floats; + + static { + NullHandling.initializeForTests(); + } + + private int index = 0; + + public TestNullableFloatColumnSelector(Float[] floats) + { + this.floats = floats; + } + + @Override + public float getFloat() + { + if (floats[index] != null) { + return floats[index]; + } else if (NullHandling.replaceWithDefault()) { + return NullHandling.ZERO_FLOAT; + } else { + throw new IllegalStateException("Should never be invoked when current value is null && SQL-compatible null handling is enabled!"); + } + } + + @Override + public boolean isNull() + { + return !NullHandling.replaceWithDefault() && floats[index] == null; + } + + public void increment() + { + ++index; + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/TestNullableLongColumnSelector.java b/processing/src/test/java/org/apache/druid/segment/TestNullableLongColumnSelector.java new file mode 100644 index 000000000000..5e27e995c784 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/TestNullableLongColumnSelector.java @@ -0,0 +1,61 @@ +/* + * 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.segment; + +import org.apache.druid.common.config.NullHandling; + +public class TestNullableLongColumnSelector extends TestLongColumnSelector +{ + private final Long[] longs; + + static { + NullHandling.initializeForTests(); + } + + private int index = 0; + + public TestNullableLongColumnSelector(Long[] longs) + { + this.longs = longs; + } + + @Override + public long getLong() + { + if (longs[index] != null) { + return longs[index]; + } else if (NullHandling.replaceWithDefault()) { + return NullHandling.ZERO_LONG; + } else { + throw new IllegalStateException("Should never be invoked when current value is null && SQL-compatible null handling is enabled!"); + } + } + + @Override + public boolean isNull() + { + return !NullHandling.replaceWithDefault() && longs[index] == null; + } + + public void increment() + { + ++index; + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/WrappingDimensionSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/WrappingDimensionSelectorTest.java new file mode 100644 index 000000000000..b208b96f81f5 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/WrappingDimensionSelectorTest.java @@ -0,0 +1,117 @@ +/* + * 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.segment; + +import org.apache.druid.common.config.NullHandling; +import org.junit.Assert; +import org.junit.Test; + +public class WrappingDimensionSelectorTest +{ + @Test + public void testLongWrappingDimensionSelector() + { + Long[] vals = new Long[]{24L, null, 50L, 0L, -60L}; + TestNullableLongColumnSelector lngSelector = new TestNullableLongColumnSelector(vals); + + LongWrappingDimensionSelector lngWrapSelector = new LongWrappingDimensionSelector(lngSelector, null); + Assert.assertEquals(24L, lngSelector.getLong()); + Assert.assertEquals("24", lngWrapSelector.getValue()); + + lngSelector.increment(); + if (NullHandling.sqlCompatible()) { + Assert.assertTrue(lngSelector.isNull()); + } else { + Assert.assertEquals(0L, lngSelector.getLong()); + } + + lngSelector.increment(); + Assert.assertEquals(50L, lngSelector.getLong()); + Assert.assertEquals("50", lngWrapSelector.getValue()); + + lngSelector.increment(); + Assert.assertEquals(0L, lngSelector.getLong()); + Assert.assertEquals("0", lngWrapSelector.getValue()); + + lngSelector.increment(); + Assert.assertEquals(-60L, lngSelector.getLong()); + Assert.assertEquals("-60", lngWrapSelector.getValue()); + } + + @Test + public void testDoubleWrappingDimensionSelector() + { + Double[] vals = new Double[]{32.0d, null, 5.0d, 0.0d, -45.0d}; + TestNullableDoubleColumnSelector dblSelector = new TestNullableDoubleColumnSelector(vals); + + DoubleWrappingDimensionSelector dblWrapSelector = new DoubleWrappingDimensionSelector(dblSelector, null); + Assert.assertEquals(32.0d, dblSelector.getDouble(), 0); + Assert.assertEquals("32.0", dblWrapSelector.getValue()); + + dblSelector.increment(); + if (NullHandling.sqlCompatible()) { + Assert.assertTrue(dblSelector.isNull()); + } else { + Assert.assertEquals(0d, dblSelector.getDouble(), 0); + } + + dblSelector.increment(); + Assert.assertEquals(5.0d, dblSelector.getDouble(), 0); + Assert.assertEquals("5.0", dblWrapSelector.getValue()); + + dblSelector.increment(); + Assert.assertEquals(0.0d, dblSelector.getDouble(), 0); + Assert.assertEquals("0.0", dblWrapSelector.getValue()); + + dblSelector.increment(); + Assert.assertEquals(-45.0d, dblSelector.getDouble(), 0); + Assert.assertEquals("-45.0", dblWrapSelector.getValue()); + } + + @Test + public void testFloatWrappingDimensionSelector() + { + Float[] vals = new Float[]{32.0f, null, 5.0f, 0.0f, -45.0f}; + TestNullableFloatColumnSelector flSelector = new TestNullableFloatColumnSelector(vals); + + FloatWrappingDimensionSelector flWrapSelector = new FloatWrappingDimensionSelector(flSelector, null); + Assert.assertEquals(32.0f, flSelector.getFloat(), 0); + Assert.assertEquals("32.0", flWrapSelector.getValue()); + + flSelector.increment(); + if (NullHandling.sqlCompatible()) { + Assert.assertTrue(flSelector.isNull()); + } else { + Assert.assertEquals(0f, flSelector.getFloat(), 0); + } + + flSelector.increment(); + Assert.assertEquals(5.0f, flSelector.getFloat(), 0); + Assert.assertEquals("5.0", flWrapSelector.getValue()); + + flSelector.increment(); + Assert.assertEquals(0.0f, flSelector.getFloat(), 0); + Assert.assertEquals("0.0", flWrapSelector.getValue()); + + flSelector.increment(); + Assert.assertEquals(-45.0f, flSelector.getFloat(), 0); + Assert.assertEquals("-45.0", flWrapSelector.getValue()); + } +} diff --git a/server/src/main/java/org/apache/druid/segment/loading/LocalDataSegmentPuller.java b/server/src/main/java/org/apache/druid/segment/loading/LocalDataSegmentPuller.java index 45121a96a074..5f63557f98eb 100644 --- a/server/src/main/java/org/apache/druid/segment/loading/LocalDataSegmentPuller.java +++ b/server/src/main/java/org/apache/druid/segment/loading/LocalDataSegmentPuller.java @@ -151,7 +151,7 @@ public FileUtils.FileCopyResult getSegmentFiles(final File sourceFile, final Fil ); } log.info( - "Coppied %d bytes from [%s] to [%s]", + "Copied %d bytes from [%s] to [%s]", result.size(), sourceFile.getAbsolutePath(), dir.getAbsolutePath()