Skip to content

Commit

Permalink
[CALCITE-4885] Fluent test fixtures so that dependent projects can wr…
Browse files Browse the repository at this point in the history
…ite parser, validator and rules tests

The goal is to be able to write tests from both inside and outside of
Calcite, and without deriving from a particular test class.

We achieve that goal by creating *fixture* objects for each kind of
test. Test configuration (e.g. what SQL query to test) belongs in that
fixture, and is set using wither methods.

Inside the fixture is a *factory* that creates the objects necessary
to run the test (parser, validator, and so forth). The factory
necssarily contains the config for the parser, validator, etc. Tests
configure the factory by calling methods in their fixture.

Also inside a fixture is a *tester* that orchestrates the lifecycle
(parse, validate, SQL-to-rel, check results). The tester is stateless
(necessary state is provided by parameters to its methods, which often
include a `SqlTestFactory`). There is one main implementation of the
tester, but it can be substituted with an radically different
implementations (say one that no-ops, or dumps all SQL expressions to
a file).

Tests that compare actual results with expected results may have a
`DiffRepository` attached to their fixture.

There follows descriptions of other refactorings included in this
change.

The code was deprecated in [CALCITE-4591], [CALCITE-4593],
[CALCITE-4446] to be removed before release 1.28.

Move classes `DiffRepository`, `MockRelOptPlanner`,
`SqlToRelTestBase`, `RelOptTestBase`, `SqlRuntimeTester`,
`SqlOperatorTest` (renaming to `CoreSqlOperatorTest`),
`SqlOperatorBaseTest` (renaming to `SqlOperatorTest`).

Class `Fixtures` is a single place to obtain fixtures to tests of the
parser, validator, operators, planner rules, and more. These fixtures
can be used within Calcite but also by any project that uses Calcite
(the project just needs to use the `testkit` module).

`class FixtureTest` tests `Fixtures` and contains examples.

Add `fixture()` methods, to create fixtures without SQL, to many test
classes including `RelToSqlConverterTest`, `InterpreterTest`,
`SqlParserTest`, `SqlValidatorTest`, `DruidAdapter2IT`,
`DruidAdapterIT`. (Previously many tests would write `sql("?")` to
create tests with dummy SQL.

In `class CalciteAssert`, move method `withRel` from `AssertQuery` to
`AssertThat`, so that tests can provide a `RelNode` without first
providing dummy SQL. Also make the list of hooks (used heavily by
`AssertQuery`) immutable.

In `SqlTester`, remove methods `checkFieldOrigin`, `checkResultType`,
`checkCollation`, `checkCharset`, `checkIntervalConv`,
`checkMonotonic`; all can now be implemented in terms of a new method
`validateAndThen`, and similar methods `validateAndApply` and
`forEachQueryValidateAndThen`.

Obsolete `interface SqlToRelTestBase.Tester`; `SqlToRelFixture` now
uses `SqlTester`.

Move inner `class MockViewExpander` from `SqlToRelTestBase` to
`SqlTestFactory`.

Method `SqlToRelTestBase.assertValid` becomes `Matchers.relIsValid`.

Refactor/rename a few methods, and change 'query' field to 'expression'.

State is now in the fixture (`class SqlParserFixture`) or
`SqlTestFactory`.

Create a fixture (`class SqlParserListFixture`) for list-based parser
tests.

Remove fixture's `transform` field; config is now transformed
immediately, not deferred.

Remove field `LINUXIFY` (thread-local state).

Fields `SqlToRelFixture.expression` and `SqlValidatorFixture.expression`
were each previously called `query` and had the opposite sense;
both are now consistent with `SqlParserFixture.expression`.

Rename method `sql` to `withSql`, `config` to `withConfig`, etc.

Rename method `withDecorrelation` to `withDecorrelate`,
`withLateDecorrelation` to `withLateDecorrelate`
`with(RelOptPlanner)` to `withPlanner`,
`with(HepProgram)` to `withProgram`.

`MaterializedViewTester` is now a utility class for materialized view
tests but is no longer required to be a base class.

Move `interface AbstractMaterializedViewTest.Sql` to top-level
`class MaterializedViewFixture`.

In `class MaterializedViewSubstitutionVisitorTest`, create a fixture
for satisfiability tests.

`RelSupplier` was previously an inner class of `class RelOptTestBase`.

It is used in metadata tests (`RelMetadataFixture`) and planner rule
tests (`RelOptFixture`) but could be used in other tests too.

Add `class RelSuppliers` with utilities and implementations for
`RelSupplier`.

Move `assertXxx` methods into a fixture, `class
SqlAdvisorTest.Fixture`.

Remove inner `class LexConfiguration`; it relied on a mutable tester
in each `SqlValidatorTest` instance, which is no longer there.

Implement methods `toString`, `equals`, `hashCode`.

Move inner `class Sql` to top-level `class SqlPrettyWriterFixture`.

Add `class ConnectionFactories`, utilities for `ConnectionFactory` and
`CalciteAssert.ConnectionPostProcessor`.

Enumerates all possible 'xxx' in JDBC `ResultSet.getXxx` and
`PreparedStatement.setXxx` methods.

Utilities for `interface SqlTester.ResultChecker`.

Move `interface SqlOperatorTest.Fixture` to `SqlOperatorFixture`.

In `class SqlOperatorFixture`, remove `double delta` arguments from
methods that test floating-point values; instead use a `ResultChecker`
created using a `ResultCheckers` method such as `isWithin` or
`isExactly`.

Remove method `withLibrary2`.

Move `class SqlToRelConverterTest.RelValidityChecker` to top-level.

In class `SqlLimitsTest`, move method `checkTypes` to
`class SqlTests`.

In a few cases we would built complex transforms (instances of
`UnaryOperator`) that were applied just before the test ran. Now we
try to apply transforms early, storing a transformed config rather
than a chained transform. It's easier to understand and debug.

Remove method `RelDataTypeSystemImpl.allowExtendedTrim`, which was
added in [CALCITE-2571] but was never used.

Remove some uses of `assert` in `TypeCoercionTest`. (Never use
`assert` in tests!)

Use `Assertions.assertEquals`, `Objects.requireNonNull` via static import.

In `class SqlValidator.Config`, rename method `sqlConformance()` to
`conformance()`, to be consistent with conformance properties
elsewhere.

In `class TryThreadLocal`, add methods `letIn(T, Supplier)` and
`letIn(T, Runnable)`.

Close apache#2685
  • Loading branch information
julianhyde committed Jan 25, 2022
1 parent 864268c commit bf56743
Show file tree
Hide file tree
Showing 132 changed files with 20,012 additions and 20,323 deletions.
95 changes: 48 additions & 47 deletions babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@
import org.apache.calcite.sql.dialect.MysqlSqlDialect;
import org.apache.calcite.sql.parser.SqlAbstractParserImpl;
import org.apache.calcite.sql.parser.SqlParser;
import org.apache.calcite.sql.parser.SqlParserImplFactory;
import org.apache.calcite.sql.parser.SqlParserFixture;
import org.apache.calcite.sql.parser.SqlParserTest;
import org.apache.calcite.sql.parser.StringAndPos;
import org.apache.calcite.sql.parser.babel.SqlBabelParserImpl;
import org.apache.calcite.tools.Hoist;

import com.google.common.base.Throwables;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

Expand All @@ -43,8 +44,10 @@
*/
class BabelParserTest extends SqlParserTest {

@Override protected SqlParserImplFactory parserImplFactory() {
return SqlBabelParserImpl.FACTORY;
@Override public SqlParserFixture fixture() {
return super.fixture()
.withTester(new BabelTesterImpl())
.withConfig(c -> c.withParserFactory(SqlBabelParserImpl.FACTORY));
}

@Test void testReservedWords() {
Expand All @@ -56,7 +59,7 @@ class BabelParserTest extends SqlParserTest {
* <p>Copy-pasted from base method, but with some key differences.
*/
@Override @Test protected void testMetadata() {
SqlAbstractParserImpl.Metadata metadata = getSqlParser("").getMetadata();
SqlAbstractParserImpl.Metadata metadata = fixture().parser().getMetadata();
assertThat(metadata.isReservedFunctionName("ABS"), is(true));
assertThat(metadata.isReservedFunctionName("FOO"), is(false));

Expand Down Expand Up @@ -195,49 +198,6 @@ class BabelParserTest extends SqlParserTest {
+ "'yyyy-MM-dd HH:mm:ss'"); // PostgreSQL gives 1969-07-20 23:00:00
}

/**
* Babel parser's global {@code LOOKAHEAD} is larger than the core
* parser's. This causes different parse error message between these two
* parsers. Here we define a looser error checker for Babel, so that we can
* reuse failure testing codes from {@link SqlParserTest}.
*
* <p>If a test case is written in this file -- that is, not inherited -- it
* is still checked by {@link SqlParserTest}'s checker.
*/
@Override protected Tester getTester() {
return new TesterImpl() {
@Override protected void checkEx(String expectedMsgPattern,
StringAndPos sap, Throwable thrown) {
if (thrownByBabelTest(thrown)) {
super.checkEx(expectedMsgPattern, sap, thrown);
} else {
checkExNotNull(sap, thrown);
}
}

private boolean thrownByBabelTest(Throwable ex) {
Throwable rootCause = Throwables.getRootCause(ex);
StackTraceElement[] stackTrace = rootCause.getStackTrace();
for (StackTraceElement stackTraceElement : stackTrace) {
String className = stackTraceElement.getClassName();
if (Objects.equals(className, BabelParserTest.class.getName())) {
return true;
}
}
return false;
}

private void checkExNotNull(StringAndPos sap,
Throwable thrown) {
if (thrown == null) {
throw new AssertionError("Expected query to throw exception, "
+ "but it did not; query [" + sap.sql
+ "]");
}
}
};
}

/** Tests parsing PostgreSQL-style "::" cast operator. */
@Test void testParseInfixCast() {
checkParseInfixCast("integer");
Expand Down Expand Up @@ -320,4 +280,45 @@ private void checkParseInfixCast(String sqlType) {
+ "and DATEADD(day, [4:DECIMAL:1], hiredate) > [5:DATE:2010-05-06]";
assertThat(hoisted.substitute(SqlParserTest::varToStr), is(expected2));
}

/**
* Babel parser's global {@code LOOKAHEAD} is larger than the core
* parser's. This causes different parse error message between these two
* parsers. Here we define a looser error checker for Babel, so that we can
* reuse failure testing codes from {@link SqlParserTest}.
*
* <p>If a test case is written in this file -- that is, not inherited -- it
* is still checked by {@link SqlParserTest}'s checker.
*/
public static class BabelTesterImpl extends TesterImpl {
@Override protected void checkEx(String expectedMsgPattern,
StringAndPos sap, @Nullable Throwable thrown) {
if (thrown != null && thrownByBabelTest(thrown)) {
super.checkEx(expectedMsgPattern, sap, thrown);
} else {
checkExNotNull(sap, thrown);
}
}

private boolean thrownByBabelTest(Throwable ex) {
Throwable rootCause = Throwables.getRootCause(ex);
StackTraceElement[] stackTrace = rootCause.getStackTrace();
for (StackTraceElement stackTraceElement : stackTrace) {
String className = stackTraceElement.getClassName();
if (Objects.equals(className, BabelParserTest.class.getName())) {
return true;
}
}
return false;
}

private void checkExNotNull(StringAndPos sap,
@Nullable Throwable thrown) {
if (thrown == null) {
throw new AssertionError("Expected query to throw exception, "
+ "but it did not; query [" + sap.sql
+ "]");
}
}
}
}
32 changes: 32 additions & 0 deletions babel/src/test/java/org/apache/calcite/test/BabelTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.calcite.test;

import org.apache.calcite.config.CalciteConnectionProperty;
import org.apache.calcite.sql.parser.SqlParserFixture;
import org.apache.calcite.sql.parser.babel.SqlBabelParserImpl;

import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -97,4 +98,35 @@ private void checkInfixCast(Statement statement, String typeName, int sqlType)
is(sqlType));
}
}

/** Tests that you can run tests via {@link Fixtures}. */
@Test void testFixtures() {
final SqlValidatorFixture v = Fixtures.forValidator();
v.withSql("select ^1 + date '2002-03-04'^")
.fails("(?s).*Cannot apply '\\+' to arguments of"
+ " type '<INTEGER> \\+ <DATE>'.*");

v.withSql("select 1 + 2 as three")
.type("RecordType(INTEGER NOT NULL THREE) NOT NULL");

// 'as' as identifier is invalid with Core parser
final SqlParserFixture p = Fixtures.forParser();
p.sql("select ^as^ from t")
.fails("(?s)Encountered \"as\".*");

// 'as' as identifier is invalid if you use Babel's tester and Core parser
p.sql("select ^as^ from t")
.withTester(new BabelParserTest.BabelTesterImpl())
.fails("(?s)Encountered \"as\".*");

// 'as' as identifier is valid with Babel parser
p.withConfig(c -> c.withParserFactory(SqlBabelParserImpl.FACTORY))
.sql("select as from t")
.ok("SELECT `AS`\n"
+ "FROM `T`");

// Postgres cast is invalid with core parser
p.sql("select 1 ^:^: integer as x")
.fails("(?s).*Encountered \":\" at .*");
}
}
28 changes: 28 additions & 0 deletions babel/src/test/java/org/apache/calcite/test/package-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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.
*/

/**
* Tests for Calcite.
*/
@DefaultQualifier(value = NonNull.class, locations = TypeUseLocation.FIELD)
@DefaultQualifier(value = NonNull.class, locations = TypeUseLocation.PARAMETER)
@DefaultQualifier(value = NonNull.class, locations = TypeUseLocation.RETURN)
package org.apache.calcite.test;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.framework.qual.DefaultQualifier;
import org.checkerframework.framework.qual.TypeUseLocation;
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import org.apache.calcite.materialize.MaterializationService;
import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.prepare.CalciteCatalogReader;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.type.DelegatingTypeSystem;
import org.apache.calcite.rel.type.RelDataTypeSystem;
import org.apache.calcite.runtime.Hook;
Expand Down Expand Up @@ -75,7 +74,6 @@
import org.checkerframework.checker.nullness.qual.Nullable;

import java.lang.reflect.Type;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.HashMap;
Expand Down Expand Up @@ -182,23 +180,10 @@ void init() {

@Override public <T> T unwrap(Class<T> iface) throws SQLException {
if (iface == RelRunner.class) {
return iface.cast(new RelRunner() {
@Override public PreparedStatement prepareStatement(RelNode rel)
throws SQLException {
return prepareStatement_(CalcitePrepare.Query.of(rel),
ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY,
getHoldability());
}

@SuppressWarnings("deprecation")
@Override public PreparedStatement prepare(RelNode rel) {
try {
return prepareStatement(rel);
} catch (SQLException e) {
throw Util.throwAsRuntime(e);
}
}
});
return iface.cast((RelRunner) rel ->
prepareStatement_(CalcitePrepare.Query.of(rel),
ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY,
getHoldability()));
}
return super.unwrap(iface);
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2052,14 +2052,14 @@ private static void registerCalcRules(RelOptPlanner planner) {

@Experimental
public static void registerDefaultRules(RelOptPlanner planner,
boolean enableMaterialziations, boolean enableBindable) {
boolean enableMaterializations, boolean enableBindable) {
if (CalciteSystemProperty.ENABLE_COLLATION_TRAIT.value()) {
registerAbstractRelationalRules(planner);
}
registerAbstractRules(planner);
registerBaseRules(planner);

if (enableMaterialziations) {
if (enableMaterializations) {
registerMaterializationRules(planner);
}
if (enableBindable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import static org.apache.calcite.linq4j.Nullness.castNonNull;
import static org.apache.calcite.util.Static.RESOURCE;

import static java.util.Objects.requireNonNull;

/**
* Shit just got real.
*
Expand Down Expand Up @@ -727,7 +728,7 @@ private static SqlValidator createSqlValidator(Context context,
final CalciteConnectionConfig connectionConfig = context.config();
final SqlValidator.Config config = SqlValidator.Config.DEFAULT
.withLenientOperatorLookup(connectionConfig.lenientOperatorLookup())
.withSqlConformance(connectionConfig.conformance())
.withConformance(connectionConfig.conformance())
.withDefaultNullCollation(connectionConfig.defaultNullCollation())
.withIdentifierExpansion(true);
return new CalciteSqlValidator(opTab, catalogReader, typeFactory,
Expand Down Expand Up @@ -1129,7 +1130,7 @@ protected SqlValidator createSqlValidator(CatalogReader catalogReader) {
internalParameters.put("_conformance", conformance);
bindable = EnumerableInterpretable.toBindable(internalParameters,
context.spark(), enumerable,
Objects.requireNonNull(prefer, "EnumerableRel.Prefer prefer"));
requireNonNull(prefer, "EnumerableRel.Prefer prefer"));
} finally {
CatalogReader.THREAD_LOCAL.remove();
}
Expand All @@ -1145,8 +1146,8 @@ protected SqlValidator createSqlValidator(CatalogReader catalogReader) {

return new PreparedResultImpl(
resultType,
Objects.requireNonNull(parameterRowType, "parameterRowType"),
Objects.requireNonNull(fieldOrigins, "fieldOrigins"),
requireNonNull(parameterRowType, "parameterRowType"),
requireNonNull(fieldOrigins, "fieldOrigins"),
root.collation.getFieldCollations().isEmpty()
? ImmutableList.of()
: ImmutableList.of(root.collation),
Expand Down Expand Up @@ -1258,7 +1259,7 @@ private static List<Expression> simpleList(BlockStatement statement) {
// Case-sensitive name match because name was previously resolved.
MemberExpression memberExpression = (MemberExpression) expression;
PseudoField field = memberExpression.field;
Expression targetExpression = Objects.requireNonNull(memberExpression.expression,
Expression targetExpression = requireNonNull(memberExpression.expression,
() -> "static field access is not implemented yet."
+ " field.name=" + field.getName()
+ ", field.declaringClass=" + field.getDeclaringClass());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ private SqlValidator createSqlValidator(CalciteCatalogReader catalogReader) {
sqlValidatorConfig
.withDefaultNullCollation(connectionConfig.defaultNullCollation())
.withLenientOperatorLookup(connectionConfig.lenientOperatorLookup())
.withSqlConformance(connectionConfig.conformance())
.withConformance(connectionConfig.conformance())
.withIdentifierExpansion(true));
}

Expand Down
65 changes: 65 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/RelValidityChecker.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* 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.calcite.rel;

import org.apache.calcite.rel.core.CorrelationId;
import org.apache.calcite.util.Litmus;

import com.google.common.collect.ImmutableSet;

import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Set;

/**
* Visitor that checks that every {@link RelNode} in a tree is valid.
*
* @see RelNode#isValid(Litmus, RelNode.Context)
*/
public class RelValidityChecker extends RelVisitor
implements RelNode.Context {
private int invalidCount;
private final Deque<RelNode> stack = new ArrayDeque<>();

@Override public Set<CorrelationId> correlationIds() {
final ImmutableSet.Builder<CorrelationId> builder =
ImmutableSet.builder();
for (RelNode r : stack) {
builder.addAll(r.getVariablesSet());
}
return builder.build();
}

@Override public void visit(RelNode node, int ordinal,
@Nullable RelNode parent) {
try {
stack.push(node);
if (!node.isValid(Litmus.THROW, this)) {
++invalidCount;
}
super.visit(node, ordinal, parent);
} finally {
stack.pop();
}
}

public int invalidCount() {
return invalidCount;
}
}
Loading

0 comments on commit bf56743

Please sign in to comment.