mirror of https://github.com/apache/druid.git
fix druid-sql issue with filtering numeric columns by null values (#9061)
* fix druid-sql issue with filtering numeric columns by null values * fix tests * fix tests for reals
This commit is contained in:
Clint Wylie
GitHub
parent
94a23fb17e
commit
84ef8b819e
|
|
@ -26,6 +26,7 @@ import org.apache.calcite.rel.type.RelDataType;
|
|||
import org.apache.calcite.rel.type.RelDataTypeFactory;
|
||||
import org.apache.calcite.rel.type.RelDataTypeField;
|
||||
import org.apache.calcite.sql.type.SqlTypeName;
|
||||
import org.apache.druid.common.config.NullHandling;
|
||||
import org.apache.druid.java.util.common.IAE;
|
||||
import org.apache.druid.java.util.common.ISE;
|
||||
import org.apache.druid.java.util.common.Pair;
|
||||
|
|
@ -146,6 +147,7 @@ public class RowSignature
|
|||
public RelDataType getRelDataType(final RelDataTypeFactory typeFactory)
|
||||
{
|
||||
final RelDataTypeFactory.Builder builder = typeFactory.builder();
|
||||
final boolean nullNumeric = !NullHandling.replaceWithDefault();
|
||||
for (final String columnName : columnNames) {
|
||||
final ValueType columnType = getColumnType(columnName);
|
||||
final RelDataType type;
|
||||
|
|
@ -159,13 +161,13 @@ public class RowSignature
|
|||
type = Calcites.createSqlTypeWithNullability(typeFactory, SqlTypeName.VARCHAR, true);
|
||||
break;
|
||||
case LONG:
|
||||
type = Calcites.createSqlType(typeFactory, SqlTypeName.BIGINT);
|
||||
type = Calcites.createSqlTypeWithNullability(typeFactory, SqlTypeName.BIGINT, nullNumeric);
|
||||
break;
|
||||
case FLOAT:
|
||||
type = Calcites.createSqlType(typeFactory, SqlTypeName.FLOAT);
|
||||
type = Calcites.createSqlTypeWithNullability(typeFactory, SqlTypeName.FLOAT, nullNumeric);
|
||||
break;
|
||||
case DOUBLE:
|
||||
type = Calcites.createSqlType(typeFactory, SqlTypeName.DOUBLE);
|
||||
type = Calcites.createSqlTypeWithNullability(typeFactory, SqlTypeName.DOUBLE, nullNumeric);
|
||||
break;
|
||||
case COMPLEX:
|
||||
// Loses information about exactly what kind of complex column this is.
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ import org.apache.calcite.avatica.AvaticaClientRuntimeException;
|
|||
import org.apache.calcite.avatica.Meta;
|
||||
import org.apache.calcite.avatica.MissingResultsException;
|
||||
import org.apache.calcite.avatica.NoSuchStatementException;
|
||||
import org.apache.druid.common.config.NullHandling;
|
||||
import org.apache.druid.guice.GuiceInjectors;
|
||||
import org.apache.druid.initialization.Initialization;
|
||||
import org.apache.druid.java.util.common.DateTimes;
|
||||
|
|
@ -118,6 +119,8 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
private static QueryRunnerFactoryConglomerate conglomerate;
|
||||
private static Closer resourceCloser;
|
||||
|
||||
private final boolean nullNumeric = !NullHandling.replaceWithDefault();
|
||||
|
||||
@BeforeClass
|
||||
public static void setUpClass()
|
||||
{
|
||||
|
|
@ -496,7 +499,7 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
Pair.of("COLUMN_NAME", "cnt"),
|
||||
Pair.of("DATA_TYPE", Types.BIGINT),
|
||||
Pair.of("TYPE_NAME", "BIGINT"),
|
||||
Pair.of("IS_NULLABLE", "NO")
|
||||
Pair.of("IS_NULLABLE", nullNumeric ? "YES" : "NO")
|
||||
),
|
||||
row(
|
||||
Pair.of("TABLE_SCHEM", "druid"),
|
||||
|
|
@ -528,7 +531,7 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
Pair.of("COLUMN_NAME", "m1"),
|
||||
Pair.of("DATA_TYPE", Types.FLOAT),
|
||||
Pair.of("TYPE_NAME", "FLOAT"),
|
||||
Pair.of("IS_NULLABLE", "NO")
|
||||
Pair.of("IS_NULLABLE", nullNumeric ? "YES" : "NO")
|
||||
),
|
||||
row(
|
||||
Pair.of("TABLE_SCHEM", "druid"),
|
||||
|
|
@ -536,7 +539,7 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
Pair.of("COLUMN_NAME", "m2"),
|
||||
Pair.of("DATA_TYPE", Types.DOUBLE),
|
||||
Pair.of("TYPE_NAME", "DOUBLE"),
|
||||
Pair.of("IS_NULLABLE", "NO")
|
||||
Pair.of("IS_NULLABLE", nullNumeric ? "YES" : "NO")
|
||||
),
|
||||
row(
|
||||
Pair.of("TABLE_SCHEM", "druid"),
|
||||
|
|
@ -587,7 +590,7 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
Pair.of("COLUMN_NAME", "cnt"),
|
||||
Pair.of("DATA_TYPE", Types.BIGINT),
|
||||
Pair.of("TYPE_NAME", "BIGINT"),
|
||||
Pair.of("IS_NULLABLE", "NO")
|
||||
Pair.of("IS_NULLABLE", nullNumeric ? "YES" : "NO")
|
||||
),
|
||||
row(
|
||||
Pair.of("TABLE_SCHEM", "druid"),
|
||||
|
|
@ -611,7 +614,7 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
Pair.of("COLUMN_NAME", "m1"),
|
||||
Pair.of("DATA_TYPE", Types.FLOAT),
|
||||
Pair.of("TYPE_NAME", "FLOAT"),
|
||||
Pair.of("IS_NULLABLE", "NO")
|
||||
Pair.of("IS_NULLABLE", nullNumeric ? "YES" : "NO")
|
||||
),
|
||||
row(
|
||||
Pair.of("TABLE_SCHEM", "druid"),
|
||||
|
|
@ -619,7 +622,7 @@ public class DruidAvaticaHandlerTest extends CalciteTestBase
|
|||
Pair.of("COLUMN_NAME", "m2"),
|
||||
Pair.of("DATA_TYPE", Types.DOUBLE),
|
||||
Pair.of("TYPE_NAME", "DOUBLE"),
|
||||
Pair.of("IS_NULLABLE", "NO")
|
||||
Pair.of("IS_NULLABLE", nullNumeric ? "YES" : "NO")
|
||||
),
|
||||
row(
|
||||
Pair.of("TABLE_SCHEM", "druid"),
|
||||
|
|
|
|||
|
|
@ -384,12 +384,12 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
ImmutableList.of(),
|
||||
ImmutableList.of(
|
||||
new Object[]{"__time", "TIMESTAMP", "NO"},
|
||||
new Object[]{"cnt", "BIGINT", "NO"},
|
||||
new Object[]{"cnt", "BIGINT", useDefault ? "NO" : "YES"},
|
||||
new Object[]{"dim1", "VARCHAR", "YES"},
|
||||
new Object[]{"dim2", "VARCHAR", "YES"},
|
||||
new Object[]{"dim3", "VARCHAR", "YES"},
|
||||
new Object[]{"m1", "FLOAT", "NO"},
|
||||
new Object[]{"m2", "DOUBLE", "NO"},
|
||||
new Object[]{"m1", "FLOAT", useDefault ? "NO" : "YES"},
|
||||
new Object[]{"m2", "DOUBLE", useDefault ? "NO" : "YES"},
|
||||
new Object[]{"unique_dim1", "OTHER", "YES"}
|
||||
)
|
||||
);
|
||||
|
|
@ -415,11 +415,11 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
ImmutableList.of(),
|
||||
ImmutableList.of(
|
||||
new Object[]{"__time", "TIMESTAMP", "NO"},
|
||||
new Object[]{"cnt", "BIGINT", "NO"},
|
||||
new Object[]{"cnt", "BIGINT", useDefault ? "NO" : "YES"},
|
||||
new Object[]{"dim1", "VARCHAR", "YES"},
|
||||
new Object[]{"dim2", "VARCHAR", "YES"},
|
||||
new Object[]{"m1", "FLOAT", "NO"},
|
||||
new Object[]{"m2", "DOUBLE", "NO"},
|
||||
new Object[]{"m1", "FLOAT", useDefault ? "NO" : "YES"},
|
||||
new Object[]{"m2", "DOUBLE", useDefault ? "NO" : "YES"},
|
||||
new Object[]{"unique_dim1", "OTHER", "YES"}
|
||||
)
|
||||
);
|
||||
|
|
@ -1705,7 +1705,16 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
.setInterval(querySegmentSpec(Filtration.eternity()))
|
||||
.setGranularity(Granularities.ALL)
|
||||
.setDimensions(dimensions(new DefaultDimensionSpec("d0", "_d0", ValueType.STRING)))
|
||||
.setAggregatorSpecs(aggregators(new CountAggregatorFactory("a0")))
|
||||
.setAggregatorSpecs(
|
||||
aggregators(
|
||||
useDefault
|
||||
? new CountAggregatorFactory("a0")
|
||||
: new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a0"),
|
||||
not(selector("d1", null, null))
|
||||
)
|
||||
)
|
||||
)
|
||||
.setHavingSpec(
|
||||
having(
|
||||
bound(
|
||||
|
|
@ -2265,6 +2274,82 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNullLongFilter() throws Exception
|
||||
{
|
||||
// bug https://github.com/apache/incubator-druid/issues/9062 prevents this test from passing
|
||||
skipVectorize();
|
||||
testQuery(
|
||||
"SELECT COUNT(*)\n"
|
||||
+ "FROM druid.numfoo\n"
|
||||
+ "WHERE l1 IS NULL",
|
||||
useDefault ? ImmutableList.of() : ImmutableList.of(
|
||||
Druids.newTimeseriesQueryBuilder()
|
||||
.dataSource(CalciteTests.DATASOURCE3)
|
||||
.intervals(querySegmentSpec(Filtration.eternity()))
|
||||
.granularity(Granularities.ALL)
|
||||
.filters(selector("l1", null, null))
|
||||
.aggregators(aggregators(new CountAggregatorFactory("a0")))
|
||||
.context(TIMESERIES_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
ImmutableList.of(
|
||||
useDefault ? new Object[]{0L} : new Object[]{3L}
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNullDoubleFilter() throws Exception
|
||||
{
|
||||
// bug https://github.com/apache/incubator-druid/issues/9062 prevents this test from passing
|
||||
skipVectorize();
|
||||
testQuery(
|
||||
"SELECT COUNT(*)\n"
|
||||
+ "FROM druid.numfoo\n"
|
||||
+ "WHERE d1 IS NULL",
|
||||
useDefault ? ImmutableList.of() : ImmutableList.of(
|
||||
Druids.newTimeseriesQueryBuilder()
|
||||
.dataSource(CalciteTests.DATASOURCE3)
|
||||
.intervals(querySegmentSpec(Filtration.eternity()))
|
||||
.granularity(Granularities.ALL)
|
||||
.filters(selector("d1", null, null))
|
||||
.aggregators(aggregators(new CountAggregatorFactory("a0")))
|
||||
.context(TIMESERIES_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
ImmutableList.of(
|
||||
useDefault ? new Object[]{0L} : new Object[]{3L}
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void testNullFloatFilter() throws Exception
|
||||
{
|
||||
// bug https://github.com/apache/incubator-druid/issues/9062 prevents this test from passing
|
||||
skipVectorize();
|
||||
testQuery(
|
||||
"SELECT COUNT(*)\n"
|
||||
+ "FROM druid.numfoo\n"
|
||||
+ "WHERE f1 IS NULL",
|
||||
useDefault ? ImmutableList.of() : ImmutableList.of(
|
||||
Druids.newTimeseriesQueryBuilder()
|
||||
.dataSource(CalciteTests.DATASOURCE3)
|
||||
.intervals(querySegmentSpec(Filtration.eternity()))
|
||||
.granularity(Granularities.ALL)
|
||||
.filters(selector("f1", null, null))
|
||||
.aggregators(aggregators(new CountAggregatorFactory("a0")))
|
||||
.context(TIMESERIES_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
ImmutableList.of(
|
||||
useDefault ? new Object[]{0L} : new Object[]{3L}
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testEmptyStringEquality() throws Exception
|
||||
{
|
||||
|
|
@ -2568,7 +2653,16 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
.dataSource(CalciteTests.DATASOURCE1)
|
||||
.intervals(querySegmentSpec(Filtration.eternity()))
|
||||
.granularity(Granularities.ALL)
|
||||
.aggregators(aggregators(new CountAggregatorFactory("a0")))
|
||||
.aggregators(
|
||||
aggregators(
|
||||
useDefault
|
||||
? new CountAggregatorFactory("a0")
|
||||
: new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a0"),
|
||||
not(selector("cnt", null, null))
|
||||
)
|
||||
)
|
||||
)
|
||||
.context(TIMESERIES_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
|
|
@ -2930,12 +3024,14 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
testQuery(
|
||||
"SELECT COUNT(*), COUNT(cnt), COUNT(dim1), AVG(cnt), SUM(cnt), SUM(cnt) + MIN(cnt) + MAX(cnt), COUNT(dim2) FROM druid.foo",
|
||||
ImmutableList.of(
|
||||
|
||||
Druids.newTimeseriesQueryBuilder()
|
||||
.dataSource(CalciteTests.DATASOURCE1)
|
||||
.intervals(querySegmentSpec(Filtration.eternity()))
|
||||
.granularity(Granularities.ALL)
|
||||
.aggregators(
|
||||
aggregators(
|
||||
useDefault
|
||||
? aggregators(
|
||||
new CountAggregatorFactory("a0"),
|
||||
new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a1"),
|
||||
|
|
@ -2951,17 +3047,40 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
not(selector("dim2", null, null))
|
||||
)
|
||||
)
|
||||
: aggregators(
|
||||
new CountAggregatorFactory("a0"),
|
||||
new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a1"),
|
||||
not(selector("cnt", null, null))
|
||||
),
|
||||
new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a2"),
|
||||
not(selector("dim1", null, null))
|
||||
),
|
||||
new LongSumAggregatorFactory("a3:sum", "cnt"),
|
||||
new CountAggregatorFactory("a3:count"),
|
||||
new LongSumAggregatorFactory("a4", "cnt"),
|
||||
new LongMinAggregatorFactory("a5", "cnt"),
|
||||
new LongMaxAggregatorFactory("a6", "cnt"),
|
||||
new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a7"),
|
||||
not(selector("dim2", null, null))
|
||||
)
|
||||
)
|
||||
)
|
||||
.postAggregators(
|
||||
new ArithmeticPostAggregator(
|
||||
"a2",
|
||||
useDefault ? "a2" : "a3",
|
||||
"quotient",
|
||||
ImmutableList.of(
|
||||
new FieldAccessPostAggregator(null, "a2:sum"),
|
||||
new FieldAccessPostAggregator(null, "a2:count")
|
||||
new FieldAccessPostAggregator(null, useDefault ? "a2:sum" : "a3:sum"),
|
||||
new FieldAccessPostAggregator(null, useDefault ? "a2:count" : "a3:count")
|
||||
)
|
||||
),
|
||||
expressionPostAgg("p0", "((\"a3\" + \"a4\") + \"a5\")")
|
||||
expressionPostAgg(
|
||||
"p0",
|
||||
useDefault ? "((\"a3\" + \"a4\") + \"a5\")" : "((\"a4\" + \"a5\") + \"a6\")"
|
||||
)
|
||||
)
|
||||
.context(TIMESERIES_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
|
|
@ -4773,9 +4892,16 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
new DefaultDimensionSpec("v0", "v0", ValueType.LONG),
|
||||
new DefaultDimensionSpec("d0", "_d0", ValueType.STRING)
|
||||
))
|
||||
.setAggregatorSpecs(aggregators(
|
||||
new CountAggregatorFactory("_a0")
|
||||
))
|
||||
.setAggregatorSpecs(
|
||||
aggregators(
|
||||
useDefault
|
||||
? new CountAggregatorFactory("_a0")
|
||||
: new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("_a0"),
|
||||
not(selector("d1", null, null))
|
||||
)
|
||||
)
|
||||
)
|
||||
.setContext(QUERY_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
|
|
@ -8031,9 +8157,16 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
new DefaultDimensionSpec("d0", "_d0", ValueType.LONG),
|
||||
new DefaultDimensionSpec("d1", "_d1", ValueType.STRING)
|
||||
))
|
||||
.setAggregatorSpecs(aggregators(
|
||||
new CountAggregatorFactory("a0")
|
||||
))
|
||||
.setAggregatorSpecs(
|
||||
aggregators(
|
||||
useDefault
|
||||
? new CountAggregatorFactory("a0")
|
||||
: new FilteredAggregatorFactory(
|
||||
new CountAggregatorFactory("a0"),
|
||||
not(selector("d2", null, null))
|
||||
)
|
||||
)
|
||||
)
|
||||
.setContext(QUERY_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
|
|
|
|||
Loading…
Reference in New Issue