Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Expressions: Fix improper supplier reuse with missing columns. (#6600) 

* Expressions: Fix improper supplier reuse with missing columns.

ExpressionSelectors has an optimization that skips building a Map
when there is only one input supplier. However, this optimization
should not be used in the case where the is one input supplier but
more than one input identifier (which can happen when only one
input identifier corresponds to an actual column).

Fixes #6556.

* Add underscores to statics.
This commit is contained in:
Gian Merlino 2018-11-15 22:13:32 -08:00 committed by GitHub
parent 93b0d58571
commit fe69da0d95
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java
View File

@ -236,7 +236,8 @@ public class ExpressionSelectors
private static Expr.ObjectBinding createBindings(Expr expression, ColumnSelectorFactory columnSelectorFactory)
{
final Map<String, Supplier<Object>> suppliers = new HashMap<>();
for (String columnName : Parser.findRequiredBindings(expression)) {
final List<String> columns = Parser.findRequiredBindings(expression);
for (String columnName : columns) {
final ColumnCapabilities columnCapabilities = columnSelectorFactory
.getColumnCapabilities(columnName);
final ValueType nativeType = columnCapabilities != null ? columnCapabilities.getType() : null;
@ -274,8 +275,9 @@ public class ExpressionSelectors
if (suppliers.isEmpty()) {
return ExprUtils.nilBindings();
} else if (suppliers.size() == 1) {
// If there's only one supplier, we can skip the Map and just use that supplier when asked for something.
} else if (suppliers.size() == 1 && columns.size() == 1) {
// If there's only one column (and it has a supplier), we can skip the Map and just use that supplier when
// asked for something.
final String column = Iterables.getOnlyElement(suppliers.keySet());
final Supplier<Object> supplier = Iterables.getOnlyElement(suppliers.values());

71
processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java
View File

@ -70,7 +70,7 @@ public class ExpressionVirtualColumnTest
ImmutableMap.of("x", 2L, "y", 3L, "z", "foobar")
);
private static final ExpressionVirtualColumn XPLUSY = new ExpressionVirtualColumn(
private static final ExpressionVirtualColumn X_PLUS_Y = new ExpressionVirtualColumn(
"expr",
"x + y",
ValueType.FLOAT,
@ -88,19 +88,25 @@ public class ExpressionVirtualColumnTest
ValueType.FLOAT,
TestExprMacroTable.INSTANCE
);
private static final ExpressionVirtualColumn ZLIKE = new ExpressionVirtualColumn(
private static final ExpressionVirtualColumn Z_LIKE = new ExpressionVirtualColumn(
"expr",
"like(z, 'f%')",
ValueType.FLOAT,
TestExprMacroTable.INSTANCE
);
private static final ExpressionVirtualColumn ZCONCATX = new ExpressionVirtualColumn(
private static final ExpressionVirtualColumn Z_CONCAT_X = new ExpressionVirtualColumn(
"expr",
"z + cast(x, 'string')",
ValueType.STRING,
TestExprMacroTable.INSTANCE
);
private static final ExpressionVirtualColumn TIMEFLOOR = new ExpressionVirtualColumn(
private static final ExpressionVirtualColumn Z_CONCAT_NONEXISTENT = new ExpressionVirtualColumn(
"expr",
"concat(z, nonexistent)",
ValueType.STRING,
TestExprMacroTable.INSTANCE
);
private static final ExpressionVirtualColumn TIME_FLOOR = new ExpressionVirtualColumn(
"expr",
"timestamp_floor(__time, 'P1D')",
ValueType.LONG,
@ -116,7 +122,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testObjectSelector()
{
final BaseObjectColumnValueSelector selector = XPLUSY.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
final BaseObjectColumnValueSelector selector = X_PLUS_Y.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
CURRENT_ROW.set(ROW0);
Assert.assertEquals(null, selector.getObject());
@ -139,7 +145,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testLongSelector()
{
final BaseLongColumnValueSelector selector = XPLUSY.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
final BaseLongColumnValueSelector selector = X_PLUS_Y.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
CURRENT_ROW.set(ROW0);
if (NullHandling.replaceWithDefault()) {
@ -166,7 +172,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testLongSelectorUsingStringFunction()
{
final BaseLongColumnValueSelector selector = ZCONCATX.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
final BaseLongColumnValueSelector selector = Z_CONCAT_X.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
CURRENT_ROW.set(ROW0);
if (NullHandling.replaceWithDefault()) {
@ -201,7 +207,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testFloatSelector()
{
final BaseFloatColumnValueSelector selector = XPLUSY.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
final BaseFloatColumnValueSelector selector = X_PLUS_Y.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
CURRENT_ROW.set(ROW0);
if (NullHandling.replaceWithDefault()) {
@ -228,7 +234,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testDimensionSelector()
{
final DimensionSelector selector = XPLUSY.makeDimensionSelector(
final DimensionSelector selector = X_PLUS_Y.makeDimensionSelector(
new DefaultDimensionSpec("expr", "expr"),
COLUMN_SELECTOR_FACTORY
);
@ -273,7 +279,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testDimensionSelectorUsingStringFunction()
{
final DimensionSelector selector = ZCONCATX.makeDimensionSelector(
final DimensionSelector selector = Z_CONCAT_X.makeDimensionSelector(
new DefaultDimensionSpec("expr", "expr"),
COLUMN_SELECTOR_FACTORY
);
@ -300,10 +306,43 @@ public class ExpressionVirtualColumnTest
Assert.assertEquals("foobar2", selector.lookupName(selector.getRow().get(0)));
}
@Test
public void testDimensionSelectorUsingNonexistentColumn()
{
final DimensionSelector selector = Z_CONCAT_NONEXISTENT.makeDimensionSelector(
new DefaultDimensionSpec("expr", "expr"),
COLUMN_SELECTOR_FACTORY
);
Assert.assertNotNull(selector);
CURRENT_ROW.set(ROW0);
Assert.assertEquals(1, selector.getRow().size());
Assert.assertNull(selector.lookupName(selector.getRow().get(0)));
CURRENT_ROW.set(ROW1);
Assert.assertEquals(1, selector.getRow().size());
Assert.assertNull(selector.lookupName(selector.getRow().get(0)));
CURRENT_ROW.set(ROW2);
Assert.assertEquals(1, selector.getRow().size());
Assert.assertEquals(
NullHandling.replaceWithDefault() ? "foobar" : null,
selector.lookupName(selector.getRow().get(0))
);
CURRENT_ROW.set(ROW3);
Assert.assertEquals(1, selector.getRow().size());
Assert.assertEquals(
NullHandling.replaceWithDefault() ? "foobar" : null,
selector.lookupName(selector.getRow().get(0))
);
}
@Test
public void testDimensionSelectorWithExtraction()
{
final DimensionSelector selector = XPLUSY.makeDimensionSelector(
final DimensionSelector selector = X_PLUS_Y.makeDimensionSelector(
new ExtractionDimensionSpec("expr", "x", new BucketExtractionFn(1.0, 0.0)),
COLUMN_SELECTOR_FACTORY
);
@ -407,7 +446,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testLongSelectorWithZLikeExprMacro()
{
final ColumnValueSelector selector = ZLIKE.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
final ColumnValueSelector selector = Z_LIKE.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
CURRENT_ROW.set(ROW0);
Assert.assertEquals(0L, selector.getLong());
@ -425,7 +464,7 @@ public class ExpressionVirtualColumnTest
@Test
public void testLongSelectorOfTimeColumn()
{
final ColumnValueSelector selector = TIMEFLOOR.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
final ColumnValueSelector selector = TIME_FLOOR.makeColumnValueSelector("expr", COLUMN_SELECTOR_FACTORY);
CURRENT_ROW.set(ROW0);
Assert.assertEquals(DateTimes.of("2000-01-01").getMillis(), selector.getLong());
@ -447,9 +486,9 @@ public class ExpressionVirtualColumnTest
@Test
public void testRequiredColumns()
{
Assert.assertEquals(ImmutableList.of("x", "y"), XPLUSY.requiredColumns());
Assert.assertEquals(ImmutableList.of("x", "y"), X_PLUS_Y.requiredColumns());
Assert.assertEquals(ImmutableList.of(), CONSTANT_LIKE.requiredColumns());
Assert.assertEquals(ImmutableList.of("z"), ZLIKE.requiredColumns());
Assert.assertEquals(ImmutableList.of("z", "x"), ZCONCATX.requiredColumns());
Assert.assertEquals(ImmutableList.of("z"), Z_LIKE.requiredColumns());
Assert.assertEquals(ImmutableList.of("z", "x"), Z_CONCAT_X.requiredColumns());
}
}