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.

Sign up for GitHub

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

Jump to bottom

tablesample handling code cleanup #5074

Merged
merged 5 commits into from Sep 4, 2020
Merged

tablesample handling code cleanup #5074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8797e61
Fix indentation
findepi Sep 3, 2020
5aaf438
Extract variable
findepi Sep 3, 2020
042216e
Add TABLESAMPLE test
findepi Sep 3, 2020
07eb8b2
Reject TABLESAMPLE when ratio expression not a number
findepi Sep 3, 2020
e19d0f1
Fix TABLESAMPLE with DECIMAL
findepi Sep 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 32 additions & 17 deletions presto-main/src/main/java/io/prestosql/sql/analyzer/StatementAnalyzer.java
View file
Open in desktop
Expand Up @@ -56,6 +56,7 @@
import io.prestosql.spi.type.Type;
import io.prestosql.spi.type.TypeNotFoundException;
import io.prestosql.spi.type.VarcharType;
import io.prestosql.sql.InterpretedFunctionInvoker;
import io.prestosql.sql.SqlPath;
import io.prestosql.sql.analyzer.Analysis.GroupingSetAnalysis;
import io.prestosql.sql.analyzer.Analysis.SelectExpression;
Expand Down Expand Up @@ -223,6 +224,7 @@
import static io.prestosql.spi.connector.StandardWarningCode.REDUNDANT_ORDER_BY;
import static io.prestosql.spi.type.BigintType.BIGINT;
import static io.prestosql.spi.type.BooleanType.BOOLEAN;
import static io.prestosql.spi.type.DoubleType.DOUBLE;
import static io.prestosql.spi.type.VarcharType.VARCHAR;
import static io.prestosql.sql.NodeUtils.getSortItemsFromOrderBy;
import static io.prestosql.sql.NodeUtils.mapFromProperties;
Expand Down Expand Up @@ -1410,13 +1412,13 @@ private Scope createScopeForMaterializedView(Table table, QualifiedObjectName na
// are implicitly coercible to the declared materialized view types.
List<Field> outputFields = viewColumns.stream()
.map(column -> Field.newQualified(
table.getName(),
Optional.of(column.getName()),
getViewColumnType(column, name, table),
false,
Optional.of(name),
Optional.of(column.getName()),
false))
table.getName(),
Optional.of(column.getName()),
getViewColumnType(column, name, table),
false,
Optional.of(name),
Optional.of(column.getName()),
false))
.collect(toImmutableList());

analysis.addRelationCoercion(table, outputFields.stream().map(Field::getType).toArray(Type[]::new));
Expand Down Expand Up @@ -1455,8 +1457,9 @@ protected Scope visitAliasedRelation(AliasedRelation relation, Optional<Scope> s
@Override
protected Scope visitSampledRelation(SampledRelation relation, Optional<Scope> scope)
{
if (!SymbolsExtractor.extractNames(relation.getSamplePercentage(), analysis.getColumnReferences()).isEmpty()) {
throw semanticException(EXPRESSION_NOT_CONSTANT, relation.getSamplePercentage(), "Sample percentage cannot contain column references");
Expression samplePercentage = relation.getSamplePercentage();
if (!SymbolsExtractor.extractNames(samplePercentage, analysis.getColumnReferences()).isEmpty()) {
throw semanticException(EXPRESSION_NOT_CONSTANT, samplePercentage, "Sample percentage cannot contain column references");
}

Map<NodeRef<Expression>, Type> expressionTypes = ExpressionAnalyzer.analyzeExpressions(
Expand All @@ -1465,29 +1468,41 @@ protected Scope visitSampledRelation(SampledRelation relation, Optional<Scope> s
accessControl,
sqlParser,
TypeProvider.empty(),
ImmutableList.of(relation.getSamplePercentage()),
ImmutableList.of(samplePercentage),
analysis.getParameters(),
WarningCollector.NOOP,
analysis.isDescribe())
.getExpressionTypes();

ExpressionInterpreter samplePercentageEval = expressionOptimizer(relation.getSamplePercentage(), metadata, session, expressionTypes);
Type samplePercentageType = expressionTypes.get(NodeRef.of(samplePercentage));
if (!typeCoercion.canCoerce(samplePercentageType, DOUBLE)) {
throw semanticException(TYPE_MISMATCH, samplePercentage, "Sample percentage should be a numeric expression");
}

ExpressionInterpreter samplePercentageEval = expressionOptimizer(samplePercentage, metadata, session, expressionTypes);

Object samplePercentageObject = samplePercentageEval.optimize(symbol -> {
throw semanticException(EXPRESSION_NOT_CONSTANT, relation.getSamplePercentage(), "Sample percentage cannot contain column references");
throw semanticException(EXPRESSION_NOT_CONSTANT, samplePercentage, "Sample percentage cannot contain column references");
});

if (!(samplePercentageObject instanceof Number)) {
throw semanticException(TYPE_MISMATCH, relation.getSamplePercentage(), "Sample percentage should evaluate to a numeric expression");
if (samplePercentageObject == null) {
throw semanticException(INVALID_ARGUMENTS, samplePercentage, "Sample percentage cannot be NULL");
}

if (samplePercentageType != DOUBLE) {
ResolvedFunction coercion = metadata.getCoercion(samplePercentageType, DOUBLE);
InterpretedFunctionInvoker functionInvoker = new InterpretedFunctionInvoker(metadata);
samplePercentageObject = functionInvoker.invoke(coercion, session.toConnectorSession(), samplePercentageObject);
verify(samplePercentageObject != null, "Coercion from %s to %s returned null", samplePercentageType, DOUBLE);
}

double samplePercentageValue = ((Number) samplePercentageObject).doubleValue();
double samplePercentageValue = (double) samplePercentageObject;

if (samplePercentageValue < 0.0) {
throw semanticException(NUMERIC_VALUE_OUT_OF_RANGE, relation.getSamplePercentage(), "Sample percentage must be greater than or equal to 0");
throw semanticException(NUMERIC_VALUE_OUT_OF_RANGE, samplePercentage, "Sample percentage must be greater than or equal to 0");
}
if ((samplePercentageValue > 100.0)) {
throw semanticException(NUMERIC_VALUE_OUT_OF_RANGE, relation.getSamplePercentage(), "Sample percentage must be less than or equal to 100");
throw semanticException(NUMERIC_VALUE_OUT_OF_RANGE, samplePercentage, "Sample percentage must be less than or equal to 100");
}

analysis.setSampleRatio(relation, samplePercentageValue / 100);
Expand Down
113 changes: 113 additions & 0 deletions presto-tests/src/test/java/io/prestosql/tests/TestTablesample.java
View file
Open in desktop
@@ -0,0 +1,113 @@
/*
* Licensed 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 io.prestosql.tests;

import com.google.common.collect.ImmutableMap;
import io.airlift.testing.Closeables;
import io.prestosql.plugin.tpch.TpchConnectorFactory;
import io.prestosql.sql.query.QueryAssertions;
import io.prestosql.testing.LocalQueryRunner;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import static io.prestosql.SessionTestUtils.TEST_SESSION;
import static io.prestosql.spi.StandardErrorCode.INVALID_ARGUMENTS;
import static io.prestosql.spi.StandardErrorCode.TYPE_MISMATCH;
import static io.prestosql.testing.assertions.PrestoExceptionAssert.assertPrestoExceptionThrownBy;
import static org.assertj.core.api.Assertions.assertThat;

public class TestTablesample
{
private LocalQueryRunner queryRunner;
private QueryAssertions assertions;

@BeforeClass
public void setUp()
throws Exception
{
queryRunner = LocalQueryRunner.create(TEST_SESSION);
queryRunner.createCatalog("tpch", new TpchConnectorFactory(1), ImmutableMap.of());
assertions = new QueryAssertions(queryRunner);
}

@AfterClass(alwaysRun = true)
public void tearDown()
throws Exception
{
Closeables.closeAll(queryRunner, assertions);
queryRunner = null;
assertions = null;
}

@Test
public void testTablesample()
{
// zero sample
assertThat(assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (0)"))
.matches("VALUES BIGINT '0'");

// full sample
assertThat(assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (100)"))
.matches("VALUES BIGINT '15000'");

// 1%
assertThat(assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (1)"))
.satisfies(result -> assertThat((Long) result.getOnlyValue()).isBetween(50L, 450L));

// 0.1%
assertThat(assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (1e-1)"))
.satisfies(result -> assertThat((Long) result.getOnlyValue()).isBetween(5L, 45L));

// 0.1% as decimal
assertThat(assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (0.1)"))
.satisfies(result -> assertThat((Long) result.getOnlyValue()).isBetween(5L, 45L));

// fraction as long decimal
assertThat(assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (0.000000000000000000001)"))
.satisfies(result -> assertThat((Long) result.getOnlyValue()).isBetween(0L, 5L));
}

@Test
public void testNullRatio()
{
// NULL
assertPrestoExceptionThrownBy(() -> assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (NULL)"))
.hasErrorCode(INVALID_ARGUMENTS)
.hasMessage("line 1:62: Sample percentage cannot be NULL");

// NULL integer
assertPrestoExceptionThrownBy(() -> assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (CAST(NULL AS integer))"))
.hasErrorCode(INVALID_ARGUMENTS)
.hasMessage("line 1:62: Sample percentage cannot be NULL");

// NULL double
assertPrestoExceptionThrownBy(() -> assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (CAST(NULL AS double))"))
.hasErrorCode(INVALID_ARGUMENTS)
.hasMessage("line 1:62: Sample percentage cannot be NULL");

// NULL varchar
assertPrestoExceptionThrownBy(() -> assertions.query("SELECT count(*) FROM tpch.tiny.orders TABLESAMPLE BERNOULLI (CAST(NULL AS varchar))"))
.hasErrorCode(TYPE_MISMATCH)
.hasMessage("line 1:62: Sample percentage should be a numeric expression");
}

@Test
public void testInvalidRatioType()
{
assertPrestoExceptionThrownBy(() -> assertions.query("SELECT count(*) FROM tpch.sf1.orders TABLESAMPLE BERNOULLI (DATE '1970-01-02')"))
.hasErrorCode(TYPE_MISMATCH)
.hasMessage("line 1:61: Sample percentage should be a numeric expression");
}
}