Skip to content

Commit

Permalink
Re-select an excluded field should throw SyntaxCheckException
Browse files Browse the repository at this point in the history
Signed-off-by: Lantao Jin <[email protected]>
  • Loading branch information
LantaoJin committed Sep 27, 2024
1 parent 0e56a46 commit 8cf092e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import org.opensearch.sql.ast.tree.Sort;
import org.opensearch.sql.ast.tree.SubqueryAlias;
import org.opensearch.sql.ast.tree.TopAggregation;
import org.opensearch.sql.common.antlr.SyntaxCheckException;
import org.opensearch.sql.ppl.utils.AggregatorTranslator;
import org.opensearch.sql.ppl.utils.BuiltinFunctionTranslator;
import org.opensearch.sql.ppl.utils.ComparatorTransformer;
Expand Down Expand Up @@ -337,7 +338,16 @@ public LogicalPlan visitAlias(Alias node, CatalystPlanContext context) {

@Override
public LogicalPlan visitProject(Project node, CatalystPlanContext context) {
if (!node.isExcluded()) {
if (node.isExcluded()) {
List<UnresolvedExpression> intersect = context.getProjectedFields().stream()
.filter(node.getProjectList()::contains)
.collect(Collectors.toList());
if (!intersect.isEmpty()) {
// Fields in parent projection, but they have be excluded in child. For example,
// source=t | fields - A, B | fields A, B, C will throw "[Field A, Field B] can't be resolved"
throw new SyntaxCheckException(intersect + " can't be resolved");
}
} else {
context.withProjectedFields(node.getProjectList());
}
LogicalPlan child = node.getChild().get(0).accept(this, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.opensearch.flint.spark.ppl

import org.opensearch.flint.spark.ppl.PlaneUtils.plan
import org.opensearch.sql.common.antlr.SyntaxCheckException
import org.opensearch.sql.ppl.{CatalystPlanContext, CatalystQueryPlanVisitor}
import org.scalatest.matchers.should.Matchers

Expand Down Expand Up @@ -311,4 +312,33 @@ class PPLLogicalPlanBasicQueriesTranslatorTestSuite
val expectedPlan = Project(Seq(UnresolvedStar(None)), planWithLimit)
comparePlans(expectedPlan, logPlan, false)
}

test("test fields + then - field list") {
val context = new CatalystPlanContext
val logPlan = planTransformer.visit(
plan(pplParser, "source=t | fields + A, B, C | fields - A, B"),
context)

val table = UnresolvedRelation(Seq("t"))
val projectABC = Project(
Seq(UnresolvedAttribute("A"), UnresolvedAttribute("B"), UnresolvedAttribute("C")),
table)
val dropList = Seq(UnresolvedAttribute("A"), UnresolvedAttribute("B"))
val dropAB = DataFrameDropColumns(dropList, projectABC)

val expectedPlan = Project(Seq(UnresolvedStar(None)), dropAB)
comparePlans(expectedPlan, logPlan, false)
}

test("test fields - then + field list") {
val context = new CatalystPlanContext
val thrown = intercept[SyntaxCheckException] {
planTransformer.visit(
plan(pplParser, "source=t | fields - A, B | fields + A, B, C"),
context)
}
assert(
thrown.getMessage
=== "[Field(field=A, fieldArgs=[]), Field(field=B, fieldArgs=[])] can't be resolved")
}
}

0 comments on commit 8cf092e

Please sign in to comment.