Skip to content

Commit

Permalink
[0.5-nexus] Disable Fields Minus in grammar to avoid data correctness…
Browse files Browse the repository at this point in the history
… issue (#732)

Signed-off-by: Lantao Jin <[email protected]>
  • Loading branch information
LantaoJin authored Oct 12, 2024
1 parent 30a77de commit af19908
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mappingClause
;

fieldsCommand
: FIELDS (PLUS | MINUS)? fieldList
: FIELDS (PLUS)? fieldList
;

renameCommand
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ public class ArgumentFactory {
* @return the list of arguments fetched from the fields command
*/
public static List<Argument> getArgumentList(OpenSearchPPLParser.FieldsCommandContext ctx) {
return Collections.singletonList(
ctx.MINUS() != null
? new Argument("exclude", new Literal(true, DataType.BOOLEAN))
: new Argument("exclude", new Literal(false, DataType.BOOLEAN)));
return Collections.singletonList(new Argument("exclude", new Literal(false, DataType.BOOLEAN)));
}

/**
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 @@ -293,4 +294,33 @@ class PPLLogicalPlanBasicQueriesTranslatorTestSuite

comparePlans(expectedPlan, logPlan, false)
}

test("for 0.5-nexus: test fields minus one field") {
val context = new CatalystPlanContext
val thrown = intercept[SyntaxCheckException] {
planTransformer.visit(plan(pplParser, "source=table | fields - A", false), context)
}

assert(thrown.getMessage.contains("Failed to parse query due to offending symbol [-]"))
}

test("for 0.5-nexus: test fields minus multiple fields") {
val context = new CatalystPlanContext
val thrown = intercept[SyntaxCheckException] {
planTransformer.visit(plan(pplParser, "source=table | fields - A, B", false), context)
}

assert(thrown.getMessage.contains("Failed to parse query due to offending symbol [-]"))
}

test("for 0.5-nexus: test fields minus with other commands") {
val context = new CatalystPlanContext
val thrown = intercept[SyntaxCheckException] {
planTransformer.visit(
plan(pplParser, "source=table | sort - A | where B > 0 | fields - C | head 1", false),
context)
}

assert(thrown.getMessage.contains("Failed to parse query due to offending symbol [-]"))
}
}

0 comments on commit af19908

Please sign in to comment.