From 5fb6c4778a0321802b582f3cab5132096ca74b8b Mon Sep 17 00:00:00 2001 From: Alex Theimer Date: Fri, 1 Mar 2024 15:27:42 -0800 Subject: [PATCH] fix(query): correcly escape LogicalPlan-to-String backslashes --- .../queryplanner/LogicalPlanParser.scala | 8 ++++++-- .../queryplanner/PlannerHierarchySpec.scala | 19 ++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/coordinator/src/main/scala/filodb.coordinator/queryplanner/LogicalPlanParser.scala b/coordinator/src/main/scala/filodb.coordinator/queryplanner/LogicalPlanParser.scala index 744014572..fe3dbd806 100644 --- a/coordinator/src/main/scala/filodb.coordinator/queryplanner/LogicalPlanParser.scala +++ b/coordinator/src/main/scala/filodb.coordinator/queryplanner/LogicalPlanParser.scala @@ -166,8 +166,12 @@ object LogicalPlanParser { def miscellaneousFnToQuery(lp: ApplyMiscellaneousFunction): String = { val prefix = s"${lp.function.entryName}$OpeningRoundBracket${convertToQuery(lp.vectors)}" - if(lp.stringArgs.isEmpty) s"$prefix$ClosingRoundBracket" - else s"$prefix$Comma${lp.stringArgs.map(Quotes + _ + Quotes).mkString(Comma)}$ClosingRoundBracket" + if (lp.stringArgs.isEmpty) { + s"$prefix$ClosingRoundBracket" + } else { + val escapedArgs = lp.stringArgs.map(Quotes + StringEscapeUtils.escapeJava(_) + Quotes) + s"$prefix$Comma${escapedArgs.mkString(Comma)}$ClosingRoundBracket" + } } def scalarVaryingDoubleToQuery(lp: ScalarVaryingDoublePlan): String = { diff --git a/coordinator/src/test/scala/filodb.coordinator/queryplanner/PlannerHierarchySpec.scala b/coordinator/src/test/scala/filodb.coordinator/queryplanner/PlannerHierarchySpec.scala index 05c32a367..df5db1ba2 100644 --- a/coordinator/src/test/scala/filodb.coordinator/queryplanner/PlannerHierarchySpec.scala +++ b/coordinator/src/test/scala/filodb.coordinator/queryplanner/PlannerHierarchySpec.scala @@ -120,15 +120,16 @@ class PlannerHierarchySpec extends AnyFunSpec with Matchers with PlanValidationS if (shardColumnFilters.exists(f => f.column == "_ns_" && f.filter.isInstanceOf[EqualsRegex])) { // to ensure that tests dont call something else that is not configured require(shardColumnFilters.exists(f => f.column == "_ns_" && f.filter.isInstanceOf[EqualsRegex] - && ( - f.filter.asInstanceOf[EqualsRegex].pattern.toString == ".*Ns" - || f.filter.asInstanceOf[EqualsRegex].pattern.toString == "localNs.*"))) + && Set(".*Ns", "localNs.*", "remoteNs.*").contains(f.filter.asInstanceOf[EqualsRegex].pattern.toString))) + val nsCol = shardColumnFilters.find(_.column == "_ns_").get if (nsCol.filter.asInstanceOf[EqualsRegex].pattern.toString == "localNs.*") { Seq( Seq(ColumnFilter("_ws_", Equals("demo")), ColumnFilter("_ns_", Equals("localNs"))), Seq(ColumnFilter("_ws_", Equals("demo")), ColumnFilter("_ns_", Equals("localNs1"))) ) + } else if (nsCol.filter.asInstanceOf[EqualsRegex].pattern.toString == "remoteNs.*") { + Seq(Seq(ColumnFilter("_ws_", Equals("demo")), ColumnFilter("_ns_", Equals("remoteNs")))) } else { Seq( Seq(ColumnFilter("_ws_", Equals("demo")), ColumnFilter("_ns_", Equals("localNs"))), @@ -233,6 +234,18 @@ class PlannerHierarchySpec extends AnyFunSpec with Matchers with PlanValidationS validatePlan(execPlan, expected) } + it("should correctly interpret escaped backslashes and add escapes for miscellaneous funcs") { + val lp = Parser.queryRangeToLogicalPlan( + """label_replace(foo{_ws_="my-ws", _ns_=~"remoteNs.*", otherFilter=~"foo\\.*"}, "newLabel", "$1", "sourceLabel", "(value-\\d+)-.*")""", + TimeStepParams(startSeconds, step, endSeconds), Antlr) + val execPlan = rootPlanner.materialize(lp, QueryContext( + origQueryParams = queryParams, + plannerParams = PlannerParams(processMultiPartition = true))) + val expected = + """E~PromQlRemoteExec(PromQlQueryParams(label_replace(foo{otherFilter=~"foo\\.*",_ws_="demo",_ns_="remoteNs"},"newLabel","$1","sourceLabel","(value-\\d+)-.*"),1633913330,300,1634777330,None,false), PlannerParams(filodb,None,None,None,None,60000,PerQueryLimits(1000000,18000000,100000,100000,300000000,1000000,200000000),PerQueryLimits(50000,15000000,50000,50000,150000000,500000,100000000),None,None,None,false,86400000,86400000,false,true,false,false,true,10,false), queryEndpoint=remotePartition-url, requestTimeoutMs=10000) on InProcessPlanDispatcher(QueryConfig(10 seconds,300000,1,50,antlr,true,true,None,Some(10000),None,None,25,true,false,true,Set(),Some(plannerSelector),Map(filodb-query-exec-metadataexec -> 65536, filodb-query-exec-aggregate-large-container -> 65536),RoutingConfig(false,1800000 milliseconds,true,0)))""".stripMargin + validatePlan(execPlan, expected) + } + it("Plan with unary expression should be equals to its binary counterpart2.") { val unaryExpressions = List("""-foo{_ws_ = "demo", _ns_ = "localNs"}""", """+foo{_ws_ = "demo", _ns_ = "localNs"}""", """-(foo{_ws_ = "demo", _ns_ = "localNs"} - bar{_ws_ = "demo", _ns_ = "localNs"})""",