You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/04/06 01:33:13 UTC

[GitHub] [druid] abhishekrb19 opened a new pull request #9622: Compaction numeric nullhandle

abhishekrb19 opened a new pull request #9622: Compaction numeric nullhandle
URL: https://github.com/apache/druid/pull/9622
 
 
   With `druid.generic.useDefaultValueForNull=false ` set and post compaction, we notice the dimensions with numeric types the original `null` value gets replaced with the default `0` value. However, dimensions of string type work as expected. This would surprise users/systems with null compliance.
   
   This issue was reported here:
   https://github.com/apache/druid/issues/8221
   
   I tested this change set by hand, but I am happy to add a test case if someone could please point me in the right direction. Looks like `CompactionTaskRunTest.java` and `CompactionTaskParallelRunTest.java` are the test files of interest?
   
   CC: @clintropolis @himanshug @gianm, thanks!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis merged pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405214289
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestWrappingDimensionSelector.java
 ##########
 @@ -0,0 +1,107 @@
+/*
+ * 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.junit.Assert;
+import org.junit.Test;
+
+public class TestWrappingDimensionSelector
+{
+  @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();
+    Assert.assertEquals(0L, lngSelector.getLong());
 
 Review comment:
   Done, that makes sense.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405214013
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestWrappingDimensionSelector.java
 ##########
 @@ -0,0 +1,107 @@
+/*
+ * 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.junit.Assert;
+import org.junit.Test;
+
+public class TestWrappingDimensionSelector
 
 Review comment:
   Ack

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405124923
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java
 ##########
 @@ -0,0 +1,55 @@
+/*
+ * 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()
+  {
+    return doubles[index] == null ? NullHandling.ZERO_DOUBLE : doubles[index];
 
 Review comment:
   Same for other column selectors.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-612133118
 
 
   Good point. Added label. Thanks!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-611836690
 
 
   @jihoonson, thanks, please see: https://github.com/apache/druid/pull/9660.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405253477
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   Would you please fix here too?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-612071872
 
 
   @jihoonson, should this/backport PR be tagged with "Area - Null Handling" label?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-611263378
 
 
   @clintropolis do you have more comments?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-610564783
 
 
   @clintropolis, thanks for the pointer! I went ahead and followed one of your suggestions to add similar selector `TestNullable*ColumnSelector` for each type that returns `null` based on the values initialized. 
   
   I verified that the newly added test cases didn't work as expected _before_ the fix in this patch, but does _after_ the fix.
   
   Could you please take a look? Thanks!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405269070
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   @jihoonson, I changed it here in the previous commit: https://github.com/apache/druid/commit/e2639e571c40069828a24be815eb67670f5aaf1e
   
   But then the CI Build Sql compatibility test failed for the Float case, please see below: 
   ```
   [ERROR] testFloatWrappingDimensionSelector(org.apache.druid.segment.WrappingDimensionSelectorTest)  Time elapsed: 0.008 s  <<< ERROR!
   java.lang.UnsupportedOperationException
   ```
   I assumed calling this was ok for `float`, but not `long` and `double` types. Please correct me if I am wrong.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405150922
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java
 ##########
 @@ -0,0 +1,55 @@
+/*
+ * 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()
+  {
+    return doubles[index] == null ? NullHandling.ZERO_DOUBLE : doubles[index];
+  }
+
+  @Override
+  public boolean isNull()
+  {
+    return doubles[index] == null;
 
 Review comment:
   Same for other column selectors.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-610708879
 
 
   @jihoonson, thank you for the review! Could you please take another pass when you get a chance? Thanks.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405269070
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   @jihoonson, I changed it here in the previous commit: https://github.com/apache/druid/commit/e2639e571c40069828a24be815eb67670f5aaf1e
   
   But then the CI Build Sql compatibility test [failed[(https://travis-ci.org/github/apache/druid/builds/672332557) for the Float case, please see below: 
   ```
   [ERROR] testFloatWrappingDimensionSelector(org.apache.druid.segment.WrappingDimensionSelectorTest)  Time elapsed: 0.008 s  <<< ERROR!
   java.lang.UnsupportedOperationException
   ```
   I assumed calling this was ok for `float`, but not `long` and `double` types. Please correct me if I am wrong.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405150798
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java
 ##########
 @@ -0,0 +1,55 @@
+/*
+ * 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()
+  {
+    return doubles[index] == null ? NullHandling.ZERO_DOUBLE : doubles[index];
+  }
+
+  @Override
+  public boolean isNull()
+  {
+    return doubles[index] == null;
 
 Review comment:
   Probably this should be `return NullHandling.replaceWithDefault() ? false : doubles[index] == null`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405827586
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   @jihoonson, ah, sorry, my bad. You're right - `float` doesn't appear to be a special case, but it was a bug in the test code. The commit above had an [extraneous call](https://github.com/apache/druid/commit/e2639e571c40069828a24be815eb67670f5aaf1e#diff-afef3502218e8fbbfbb4898cc9f88972R99) because of which the test failed.
   
   I pushed the corrected changes in my latest commit. Sorry about the confusion. Could you please take another look?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405128094
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestWrappingDimensionSelector.java
 ##########
 @@ -0,0 +1,107 @@
+/*
+ * 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.junit.Assert;
+import org.junit.Test;
+
+public class TestWrappingDimensionSelector
 
 Review comment:
   We usually append "Test" at end of the class name instead of the beginning.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405151099
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestWrappingDimensionSelector.java
 ##########
 @@ -0,0 +1,107 @@
+/*
+ * 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.junit.Assert;
+import org.junit.Test;
+
+public class TestWrappingDimensionSelector
+{
+  @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();
+    Assert.assertEquals(0L, lngSelector.getLong());
 
 Review comment:
   `lngSelector.getLong` should be never called when the current value is null and `NullHandling.sqlCompatible() == true`. Probably better to check
   
   ```java
   if (NullHandling.sqlCompatible()) {
     Assert.assertTrue(lngSelector.isNull());
   } else {
     Assert.assertEquals(0L, lngSelector.getLong());
   }

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405269070
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   @jihoonson, I changed it here in the previous commit: https://github.com/apache/druid/commit/e2639e571c40069828a24be815eb67670f5aaf1e
   
   But then the CI Build Sql compatibility test [failed](https://travis-ci.org/github/apache/druid/builds/672332557) for the Float case, please see below: 
   ```
   [ERROR] testFloatWrappingDimensionSelector(org.apache.druid.segment.WrappingDimensionSelectorTest)  Time elapsed: 0.008 s  <<< ERROR!
   java.lang.UnsupportedOperationException
   ```
   I assumed calling `getFloat()` was ok after reading through some druid sql compatibility documentation. Please correct me if I am wrong.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405124829
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java
 ##########
 @@ -0,0 +1,55 @@
+/*
+ * 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()
+  {
+    return doubles[index] == null ? NullHandling.ZERO_DOUBLE : doubles[index];
 
 Review comment:
   Probably this should return 0 when `doubles[index] == null && NullHandling.replaceWithDefault() == true`? Otherwise it probably should throw an exception.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-611823427
 
 
   @jihoonson and @clintropolis, thanks for the reviews! I am not familiar with the release branching strategy, but will this patch be in `0.18.0`? Since this is not in `0.17.0`, this doesn't show up in https://github.com/apache/druid/compare/0.17.0...0.18.0. Should this be backported or tagged with a milestone? Thanks!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405214107
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java
 ##########
 @@ -0,0 +1,55 @@
+/*
+ * 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()
+  {
+    return doubles[index] == null ? NullHandling.ZERO_DOUBLE : doubles[index];
+  }
+
+  @Override
+  public boolean isNull()
+  {
+    return doubles[index] == null;
 
 Review comment:
   Done.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405254175
 
 

 ##########
 File path: 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 UnsupportedOperationException();
 
 Review comment:
   nit: I don't think it matters much, but perhaps `NullPointerException` or `InvalidStateException` would be more appropriate.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#issuecomment-611824682
 
 
   I just tagged this PR for 0.18.0 since it's a cool bug fix 🙂. @abhishekrb19 could you create a backport PR for 0.18.0?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405213973
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableDoubleColumnSelector.java
 ##########
 @@ -0,0 +1,55 @@
+/*
+ * 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()
+  {
+    return doubles[index] == null ? NullHandling.ZERO_DOUBLE : doubles[index];
 
 Review comment:
   Done.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
abhishekrb19 commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405269070
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   @jihoonson, I changed it here in the previous commit: https://github.com/apache/druid/commit/e2639e571c40069828a24be815eb67670f5aaf1e
   
   But then the CI Build Sql compatibility test [failed](https://travis-ci.org/github/apache/druid/builds/672332557) for the Float case, please see below: 
   ```
   [ERROR] testFloatWrappingDimensionSelector(org.apache.druid.segment.WrappingDimensionSelectorTest)  Time elapsed: 0.008 s  <<< ERROR!
   java.lang.UnsupportedOperationException
   ```
   I assumed calling this was ok for `float`, but not `long` and `double` types. Please correct me if I am wrong.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9622: Preserve the null values for numeric type dimensions post-compaction.
URL: https://github.com/apache/druid/pull/9622#discussion_r405803078
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/segment/TestNullableFloatColumnSelector.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * 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 {
+      return NullHandling.ZERO_FLOAT;
 
 Review comment:
   I'm not sure whether float is special or not. Would you point out the doc you read?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org