Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix auto type column indexer realtime logical type to report long if all nulls since it is most restrictive type #16828

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,10 @@ public ColumnType getLogicalType()
return ColumnType.NESTED_DATA;
}
if (isConstant && constantValue == null) {
// we didn't see anything, so we can be anything, so why not a string?
return ColumnType.STRING;
// we didn't see anything, so we can be anything, so be long as it is the most restrictive type in SQL compatible
// mode. Default value mode however doesn't allow null numeric values to exist, so we're going to be a STRING
// since they can have nulls
return NullHandling.sqlCompatible() ? ColumnType.LONG : ColumnType.STRING;
}
if (fieldIndexers.size() == 1 && fieldIndexers.containsKey(NestedPathFinder.JSON_PATH_ROOT)) {
FieldIndexer rootField = fieldIndexers.get(NestedPathFinder.JSON_PATH_ROOT);
Expand Down Expand Up @@ -553,6 +555,9 @@ private ColumnValueSelector<?> getRootLiteralValueSelector(
if (fieldIndexers.size() > 1 || hasNestedData) {
return null;
}
if (isConstant && constantValue == null) {
return NilColumnValueSelector.instance();
}
final FieldIndexer root = fieldIndexers.get(NestedPathFinder.JSON_PATH_ROOT);
if (root == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.io.Closer;
import org.apache.druid.java.util.common.logger.Logger;
Expand Down Expand Up @@ -93,7 +94,6 @@ public AutoTypeColumnMerger(
Closer closer
)
{

this.name = name;
this.castToType = castToType;
this.indexSpec = indexSpec;
Expand Down Expand Up @@ -159,17 +159,28 @@ public void writeMergedValueDictionary(List<IndexableAdapter> adapters) throws I
explicitType = null;
}

// for backwards compat; remove this constant handling in druid 28 along with
// indexSpec.optimizeJsonConstantColumns in favor of always writing constant columns
// we also handle the numMergeIndex == 0 here, which also indicates that the column is a null constant
// null constant case. we also handle the numMergeIndex == 0 here, which also indicates that the column is a null
// constant
if (explicitType == null && !forceNested && ((isConstant && constantValue == null) || numMergeIndex == 0)) {
logicalType = ColumnType.STRING;
serializer = new ScalarStringColumnSerializer(
name,
indexSpec,
segmentWriteOutMedium,
closer
);
// if sql compatible mode, use LONG since it is the most restrictive type. Default value mode however must
// write out a STRING since null values do not exist for numeric types
if (NullHandling.sqlCompatible()) {
logicalType = ColumnType.LONG;
serializer = new ScalarLongColumnSerializer(
name,
indexSpec,
segmentWriteOutMedium,
closer
);
} else {
logicalType = ColumnType.STRING;
serializer = new ScalarStringColumnSerializer(
name,
indexSpec,
segmentWriteOutMedium,
closer
);
}
} else if (explicitType != null || (!forceNested && rootOnly && rootTypes.getSingleType() != null)) {
logicalType = explicitType != null ? explicitType : rootTypes.getSingleType();
// empty arrays can be missed since they don't have a type, so handle them here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ public void testConstantNull()
Assert.assertTrue(indexer.hasNulls);
Assert.assertFalse(indexer.hasNestedData);
Assert.assertTrue(indexer.isConstant());
Assert.assertEquals(ColumnType.STRING, indexer.getLogicalType());
Assert.assertEquals(ColumnType.LONG, indexer.getLogicalType());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6567,7 +6567,7 @@ public void testScanAllTypesAuto()
.add("arrayBool", ColumnType.LONG_ARRAY)
.add("arrayNestedLong", ColumnType.NESTED_DATA)
.add("arrayObject", ColumnType.NESTED_DATA)
.add("null", ColumnType.STRING)
.add("null", NullHandling.sqlCompatible() ? ColumnType.LONG : ColumnType.STRING)
.add("cstr", ColumnType.STRING)
.add("clong", ColumnType.LONG)
.add("cdouble", ColumnType.DOUBLE)
Expand Down
Loading