From 3e6ef2e8a435a91b6a76876e9833917e5aa0945e Mon Sep 17 00:00:00 2001 From: petermaxlee Date: Thu, 18 Aug 2016 16:17:01 +0800 Subject: [SPARK-17034][SQL] Minor code cleanup for UnresolvedOrdinal ## What changes were proposed in this pull request? I was looking at the code for UnresolvedOrdinal and made a few small changes to make it slightly more clear: 1. Rename the rule to SubstituteUnresolvedOrdinals which is more consistent with other rules that start with verbs. Note that this is still inconsistent with CTESubstitution and WindowsSubstitution. 2. Broke the test suite down from a single test case to three test cases. ## How was this patch tested? This is a minor cleanup. Author: petermaxlee Closes #14672 from petermaxlee/SPARK-17034. --- .../SubstituteUnresolvedOrdinalsSuite.scala | 67 ++++++++++++++++++++++ .../UnresolvedOrdinalSubstitutionSuite.scala | 65 --------------------- 2 files changed, 67 insertions(+), 65 deletions(-) create mode 100644 sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinalsSuite.scala delete mode 100644 sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitutionSuite.scala (limited to 'sql/catalyst/src/test') diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinalsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinalsSuite.scala new file mode 100644 index 0000000000..3c429ebce1 --- /dev/null +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinalsSuite.scala @@ -0,0 +1,67 @@ +/* + * 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.spark.sql.catalyst.analysis + +import org.apache.spark.sql.catalyst.analysis.TestRelations.testRelation2 +import org.apache.spark.sql.catalyst.dsl.expressions._ +import org.apache.spark.sql.catalyst.dsl.plans._ +import org.apache.spark.sql.catalyst.expressions.Literal +import org.apache.spark.sql.catalyst.SimpleCatalystConf + +class SubstituteUnresolvedOrdinalsSuite extends AnalysisTest { + private lazy val conf = SimpleCatalystConf(caseSensitiveAnalysis = true) + private lazy val a = testRelation2.output(0) + private lazy val b = testRelation2.output(1) + + test("unresolved ordinal should not be unresolved") { + // Expression OrderByOrdinal is unresolved. + assert(!UnresolvedOrdinal(0).resolved) + } + + test("order by ordinal") { + // Tests order by ordinal, apply single rule. + val plan = testRelation2.orderBy(Literal(1).asc, Literal(2).asc) + comparePlans( + new SubstituteUnresolvedOrdinals(conf).apply(plan), + testRelation2.orderBy(UnresolvedOrdinal(1).asc, UnresolvedOrdinal(2).asc)) + + // Tests order by ordinal, do full analysis + checkAnalysis(plan, testRelation2.orderBy(a.asc, b.asc)) + + // order by ordinal can be turned off by config + comparePlans( + new SubstituteUnresolvedOrdinals(conf.copy(orderByOrdinal = false)).apply(plan), + testRelation2.orderBy(Literal(1).asc, Literal(2).asc)) + } + + test("group by ordinal") { + // Tests group by ordinal, apply single rule. + val plan2 = testRelation2.groupBy(Literal(1), Literal(2))('a, 'b) + comparePlans( + new SubstituteUnresolvedOrdinals(conf).apply(plan2), + testRelation2.groupBy(UnresolvedOrdinal(1), UnresolvedOrdinal(2))('a, 'b)) + + // Tests group by ordinal, do full analysis + checkAnalysis(plan2, testRelation2.groupBy(a, b)(a, b)) + + // group by ordinal can be turned off by config + comparePlans( + new SubstituteUnresolvedOrdinals(conf.copy(groupByOrdinal = false)).apply(plan2), + testRelation2.groupBy(Literal(1), Literal(2))('a, 'b)) + } +} diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitutionSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitutionSuite.scala deleted file mode 100644 index 23995e96e1..0000000000 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitutionSuite.scala +++ /dev/null @@ -1,65 +0,0 @@ -/* - * 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.spark.sql.catalyst.analysis - -import org.apache.spark.sql.catalyst.analysis.TestRelations.testRelation2 -import org.apache.spark.sql.catalyst.dsl.expressions._ -import org.apache.spark.sql.catalyst.dsl.plans._ -import org.apache.spark.sql.catalyst.expressions.Literal -import org.apache.spark.sql.catalyst.SimpleCatalystConf - -class UnresolvedOrdinalSubstitutionSuite extends AnalysisTest { - - test("test rule UnresolvedOrdinalSubstitution, replaces ordinal in order by or group by") { - val a = testRelation2.output(0) - val b = testRelation2.output(1) - val conf = new SimpleCatalystConf(caseSensitiveAnalysis = true) - - // Expression OrderByOrdinal is unresolved. - assert(!UnresolvedOrdinal(0).resolved) - - // Tests order by ordinal, apply single rule. - val plan = testRelation2.orderBy(Literal(1).asc, Literal(2).asc) - comparePlans( - new UnresolvedOrdinalSubstitution(conf).apply(plan), - testRelation2.orderBy(UnresolvedOrdinal(1).asc, UnresolvedOrdinal(2).asc)) - - // Tests order by ordinal, do full analysis - checkAnalysis(plan, testRelation2.orderBy(a.asc, b.asc)) - - // order by ordinal can be turned off by config - comparePlans( - new UnresolvedOrdinalSubstitution(conf.copy(orderByOrdinal = false)).apply(plan), - testRelation2.orderBy(Literal(1).asc, Literal(2).asc)) - - - // Tests group by ordinal, apply single rule. - val plan2 = testRelation2.groupBy(Literal(1), Literal(2))('a, 'b) - comparePlans( - new UnresolvedOrdinalSubstitution(conf).apply(plan2), - testRelation2.groupBy(UnresolvedOrdinal(1), UnresolvedOrdinal(2))('a, 'b)) - - // Tests group by ordinal, do full analysis - checkAnalysis(plan2, testRelation2.groupBy(a, b)(a, b)) - - // group by ordinal can be turned off by config - comparePlans( - new UnresolvedOrdinalSubstitution(conf.copy(groupByOrdinal = false)).apply(plan2), - testRelation2.groupBy(Literal(1), Literal(2))('a, 'b)) - } -} -- cgit v1.2.3