Repository: spark
Updated Branches:
refs/heads/master 15c2bd01b -> faabe69cc
[SPARK-18952] Regex strings not properly escaped in codegen for aggregations
## What changes were proposed in this pull request?
If I use the function regexp_extract, and then in my regex string, use `\`,
i.e. escape character, this fails codegen, because the `\` character is not
properly escaped when codegen'd.
Example stack trace:
```
/* 059 */ private int maxSteps = 2;
/* 060 */ private int numRows = 0;
/* 061 */ private org.apache.spark.sql.types.StructType keySchema = new
org.apache.spark.sql.types.StructType().add("date_format(window#325.start,
yyyy-MM-dd HH:mm)", org.apache.spark.sql.types.DataTypes.StringType)
/* 062 */ .add("regexp_extract(source#310.description, ([a-zA-Z]+)\[.*,
1)", org.apache.spark.sql.types.DataTypes.StringType);
/* 063 */ private org.apache.spark.sql.types.StructType valueSchema = new
org.apache.spark.sql.types.StructType().add("sum",
org.apache.spark.sql.types.DataTypes.LongType);
/* 064 */ private Object emptyVBase;
...
org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 62,
Column 58: Invalid escape sequence
at org.codehaus.janino.Scanner.scanLiteralCharacter(Scanner.java:918)
at org.codehaus.janino.Scanner.produce(Scanner.java:604)
at org.codehaus.janino.Parser.peekRead(Parser.java:3239)
at org.codehaus.janino.Parser.parseArguments(Parser.java:3055)
at org.codehaus.janino.Parser.parseSelector(Parser.java:2914)
at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:2617)
at
org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:2573)
at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:2552)
```
In the codegend expression, the literal should use `\\` instead of `\`
A similar problem was solved here: https://github.com/apache/spark/pull/15156.
## How was this patch tested?
Regression test in `DataFrameAggregationSuite`
Author: Burak Yavuz <[email protected]>
Closes #16361 from brkyvz/reg-break.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/faabe69c
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/faabe69c
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/faabe69c
Branch: refs/heads/master
Commit: faabe69cc081145f43f9c68db1a7a8c5c39684fb
Parents: 15c2bd0
Author: Burak Yavuz <[email protected]>
Authored: Mon Jan 9 14:25:38 2017 -0800
Committer: Josh Rosen <[email protected]>
Committed: Mon Jan 9 14:25:38 2017 -0800
----------------------------------------------------------------------
.../execution/aggregate/RowBasedHashMapGenerator.scala | 12 +++++++-----
.../aggregate/VectorizedHashMapGenerator.scala | 12 +++++++-----
.../org/apache/spark/sql/DataFrameAggregateSuite.scala | 9 +++++++++
3 files changed, 23 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/faabe69c/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala
----------------------------------------------------------------------
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala
index a77e178..9316ebc 100644
---
a/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala
+++
b/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala
@@ -43,28 +43,30 @@ class RowBasedHashMapGenerator(
extends HashMapGenerator (ctx, aggregateExpressions, generatedClassName,
groupingKeySchema, bufferSchema) {
- protected def initializeAggregateHashMap(): String = {
+ override protected def initializeAggregateHashMap(): String = {
val generatedKeySchema: String =
s"new org.apache.spark.sql.types.StructType()" +
groupingKeySchema.map { key =>
+ val keyName = ctx.addReferenceMinorObj(key.name)
key.dataType match {
case d: DecimalType =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.createDecimalType(
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.createDecimalType(
|${d.precision}, ${d.scale}))""".stripMargin
case _ =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
}
}.mkString("\n").concat(";")
val generatedValueSchema: String =
s"new org.apache.spark.sql.types.StructType()" +
bufferSchema.map { key =>
+ val keyName = ctx.addReferenceMinorObj(key.name)
key.dataType match {
case d: DecimalType =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.createDecimalType(
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.createDecimalType(
|${d.precision}, ${d.scale}))""".stripMargin
case _ =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
}
}.mkString("\n").concat(";")
http://git-wip-us.apache.org/repos/asf/spark/blob/faabe69c/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/VectorizedHashMapGenerator.scala
----------------------------------------------------------------------
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/VectorizedHashMapGenerator.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/VectorizedHashMapGenerator.scala
index 7418df9..0c40417 100644
---
a/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/VectorizedHashMapGenerator.scala
+++
b/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/VectorizedHashMapGenerator.scala
@@ -48,28 +48,30 @@ class VectorizedHashMapGenerator(
extends HashMapGenerator (ctx, aggregateExpressions, generatedClassName,
groupingKeySchema, bufferSchema) {
- protected def initializeAggregateHashMap(): String = {
+ override protected def initializeAggregateHashMap(): String = {
val generatedSchema: String =
s"new org.apache.spark.sql.types.StructType()" +
(groupingKeySchema ++ bufferSchema).map { key =>
+ val keyName = ctx.addReferenceMinorObj(key.name)
key.dataType match {
case d: DecimalType =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.createDecimalType(
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.createDecimalType(
|${d.precision}, ${d.scale}))""".stripMargin
case _ =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
}
}.mkString("\n").concat(";")
val generatedAggBufferSchema: String =
s"new org.apache.spark.sql.types.StructType()" +
bufferSchema.map { key =>
+ val keyName = ctx.addReferenceMinorObj(key.name)
key.dataType match {
case d: DecimalType =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.createDecimalType(
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.createDecimalType(
|${d.precision}, ${d.scale}))""".stripMargin
case _ =>
- s""".add("${key.name}",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
+ s""".add("$keyName",
org.apache.spark.sql.types.DataTypes.${key.dataType})"""
}
}.mkString("\n").concat(";")
http://git-wip-us.apache.org/repos/asf/spark/blob/faabe69c/sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala
----------------------------------------------------------------------
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala
index 6451759..7853b22fe 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala
@@ -97,6 +97,15 @@ class DataFrameAggregateSuite extends QueryTest with
SharedSQLContext {
)
}
+ test("SPARK-18952: regexes fail codegen when used as keys due to bad
forward-slash escapes") {
+ val df = Seq(("some[thing]", "random-string")).toDF("key", "val")
+
+ checkAnswer(
+ df.groupBy(regexp_extract('key, "([a-z]+)\\[", 1)).count(),
+ Row("some", 1) :: Nil
+ )
+ }
+
test("rollup") {
checkAnswer(
courseSales.rollup("course", "year").sum("earnings"),
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
