From da07ad9c79a876caa174288056ae606390881087 Mon Sep 17 00:00:00 2001 From: Srinath Shankar Date: Wed, 17 Aug 2016 14:22:55 -0700 Subject: [PATCH 1/3] [SC-4296][SQL] Change error message for out of range numeric literals Modifies the error message produced when numeric literals are out of range. Message produced is now of the form Numeric literal does not fit in range [min, max] for type --- .../sql/catalyst/parser/AstBuilder.scala | 29 ++++++++++++------- .../sql-tests/results/literals.sql.out | 6 ++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index 283e4d43ba2b9..347cab7bf74f1 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -1278,10 +1278,17 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { } /** Create a numeric literal expression. */ - private def numericLiteral(ctx: NumberContext)(f: String => Any): Literal = withOrigin(ctx) { - val raw = ctx.getText + private def numericLiteral(ctx: NumberContext) + (minValue: BigDecimal, maxValue: BigDecimal, typeName: String) + (f: String => Any): Literal = withOrigin(ctx) { + val rawStrippedQualifier = ctx.getText.substring(0, ctx.getText.length - 1) try { - Literal(f(raw.substring(0, raw.length - 1))) + val rawBigDecimal = BigDecimal(rawStrippedQualifier) + if (rawBigDecimal < minValue || rawBigDecimal > maxValue) { + throw new ParseException(s"Numeric literal ${rawStrippedQualifier} does not " + + s"fit in range [${minValue}, ${maxValue}] for type ${typeName}", ctx) + } + Literal(f(rawStrippedQualifier)) } catch { case e: NumberFormatException => throw new ParseException(e.getMessage, ctx) @@ -1291,29 +1298,29 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { /** * Create a Byte Literal expression. */ - override def visitTinyIntLiteral(ctx: TinyIntLiteralContext): Literal = numericLiteral(ctx) { - _.toByte + override def visitTinyIntLiteral(ctx: TinyIntLiteralContext): Literal = { + numericLiteral(ctx)(Byte.MinValue, Byte.MaxValue, ByteType.simpleString)(_.toByte) } /** * Create a Short Literal expression. */ - override def visitSmallIntLiteral(ctx: SmallIntLiteralContext): Literal = numericLiteral(ctx) { - _.toShort + override def visitSmallIntLiteral(ctx: SmallIntLiteralContext): Literal = { + numericLiteral(ctx)(Short.MinValue, Short.MaxValue, ShortType.simpleString)(_.toShort) } /** * Create a Long Literal expression. */ - override def visitBigIntLiteral(ctx: BigIntLiteralContext): Literal = numericLiteral(ctx) { - _.toLong + override def visitBigIntLiteral(ctx: BigIntLiteralContext): Literal = { + numericLiteral(ctx)(Long.MinValue, Long.MaxValue, LongType.simpleString)(_.toLong) } /** * Create a Double Literal expression. */ - override def visitDoubleLiteral(ctx: DoubleLiteralContext): Literal = numericLiteral(ctx) { - _.toDouble + override def visitDoubleLiteral(ctx: DoubleLiteralContext): Literal = { + numericLiteral(ctx)(Double.MinValue, Double.MaxValue, DoubleType.simpleString)(_.toDouble) } /** diff --git a/sql/core/src/test/resources/sql-tests/results/literals.sql.out b/sql/core/src/test/resources/sql-tests/results/literals.sql.out index b964a6fc0921f..67e6d78dfbf24 100644 --- a/sql/core/src/test/resources/sql-tests/results/literals.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/literals.sql.out @@ -41,7 +41,7 @@ struct<> -- !query 4 output org.apache.spark.sql.catalyst.parser.ParseException -Value out of range. Value:"128" Radix:10(line 1, pos 7) +Numeric literal 128 does not fit in range [-128, 127] for type tinyint(line 1, pos 7) == SQL == select 128Y @@ -71,7 +71,7 @@ struct<> -- !query 7 output org.apache.spark.sql.catalyst.parser.ParseException -Value out of range. Value:"32768" Radix:10(line 1, pos 7) +Numeric literal 32768 does not fit in range [-32768, 32767] for type smallint(line 1, pos 7) == SQL == select 32768S @@ -101,7 +101,7 @@ struct<> -- !query 10 output org.apache.spark.sql.catalyst.parser.ParseException -For input string: "9223372036854775808"(line 1, pos 7) +Numeric literal 9223372036854775808 does not fit in range [-9223372036854775808, 9223372036854775807] for type bigint(line 1, pos 7) == SQL == select 9223372036854775808L From 89a73e3cda4735e0c221b02ec996525ba9cb6881 Mon Sep 17 00:00:00 2001 From: Srinath Shankar Date: Fri, 19 Aug 2016 13:37:27 -0700 Subject: [PATCH 2/3] Fixed organization of parameters into lists Added error message to ExpressionParserSuite Style fixes --- .../spark/sql/catalyst/parser/AstBuilder.scala | 16 ++++++++-------- .../catalyst/parser/ExpressionParserSuite.scala | 7 ++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index 347cab7bf74f1..8b98efcbf33c8 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -1278,9 +1278,9 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { } /** Create a numeric literal expression. */ - private def numericLiteral(ctx: NumberContext) - (minValue: BigDecimal, maxValue: BigDecimal, typeName: String) - (f: String => Any): Literal = withOrigin(ctx) { + private def numericLiteral + (ctx: NumberContext, minValue: BigDecimal, maxValue: BigDecimal, typeName: String) + (converter: String => Any): Literal = withOrigin(ctx) { val rawStrippedQualifier = ctx.getText.substring(0, ctx.getText.length - 1) try { val rawBigDecimal = BigDecimal(rawStrippedQualifier) @@ -1288,7 +1288,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { throw new ParseException(s"Numeric literal ${rawStrippedQualifier} does not " + s"fit in range [${minValue}, ${maxValue}] for type ${typeName}", ctx) } - Literal(f(rawStrippedQualifier)) + Literal(converter(rawStrippedQualifier)) } catch { case e: NumberFormatException => throw new ParseException(e.getMessage, ctx) @@ -1299,28 +1299,28 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { * Create a Byte Literal expression. */ override def visitTinyIntLiteral(ctx: TinyIntLiteralContext): Literal = { - numericLiteral(ctx)(Byte.MinValue, Byte.MaxValue, ByteType.simpleString)(_.toByte) + numericLiteral(ctx, Byte.MinValue, Byte.MaxValue, ByteType.simpleString)(_.toByte) } /** * Create a Short Literal expression. */ override def visitSmallIntLiteral(ctx: SmallIntLiteralContext): Literal = { - numericLiteral(ctx)(Short.MinValue, Short.MaxValue, ShortType.simpleString)(_.toShort) + numericLiteral(ctx, Short.MinValue, Short.MaxValue, ShortType.simpleString)(_.toShort) } /** * Create a Long Literal expression. */ override def visitBigIntLiteral(ctx: BigIntLiteralContext): Literal = { - numericLiteral(ctx)(Long.MinValue, Long.MaxValue, LongType.simpleString)(_.toLong) + numericLiteral(ctx, Long.MinValue, Long.MaxValue, LongType.simpleString)(_.toLong) } /** * Create a Double Literal expression. */ override def visitDoubleLiteral(ctx: DoubleLiteralContext): Literal = { - numericLiteral(ctx)(Double.MinValue, Double.MaxValue, DoubleType.simpleString)(_.toDouble) + numericLiteral(ctx, Double.MinValue, Double.MaxValue, DoubleType.simpleString)(_.toDouble) } /** diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala index 849d96212822c..450e9ece0d3f7 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala @@ -375,15 +375,16 @@ class ExpressionParserSuite extends PlanTest { // Tiny Int Literal assertEqual("10Y", Literal(10.toByte)) - intercept("-1000Y") + intercept("-1000Y", s"does not fit in range [-${Byte.MinValue}, ${Byte.MaxValue}]") // Small Int Literal assertEqual("10S", Literal(10.toShort)) - intercept("40000S") + intercept("40000S", s"does not fit in range [-${Short.MinValue}, ${Short.MaxValue}]") // Long Int Literal assertEqual("10L", Literal(10L)) - intercept("78732472347982492793712334L") + intercept("78732472347982492793712334L", + s"does not fit in range [-${Long.MinValue}, ${Long.MaxValue}]") // Double Literal assertEqual("10.0D", Literal(10.0D)) From 19582ff633932c3ec0a6804bec9314a3390a6404 Mon Sep 17 00:00:00 2001 From: Srinath Shankar Date: Fri, 19 Aug 2016 17:14:30 -0700 Subject: [PATCH 3/3] Fixed tests and added expression parser test for double --- .../spark/sql/catalyst/parser/ExpressionParserSuite.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala index 450e9ece0d3f7..401d9cd9d288c 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala @@ -375,19 +375,21 @@ class ExpressionParserSuite extends PlanTest { // Tiny Int Literal assertEqual("10Y", Literal(10.toByte)) - intercept("-1000Y", s"does not fit in range [-${Byte.MinValue}, ${Byte.MaxValue}]") + intercept("-1000Y", s"does not fit in range [${Byte.MinValue}, ${Byte.MaxValue}]") // Small Int Literal assertEqual("10S", Literal(10.toShort)) - intercept("40000S", s"does not fit in range [-${Short.MinValue}, ${Short.MaxValue}]") + intercept("40000S", s"does not fit in range [${Short.MinValue}, ${Short.MaxValue}]") // Long Int Literal assertEqual("10L", Literal(10L)) intercept("78732472347982492793712334L", - s"does not fit in range [-${Long.MinValue}, ${Long.MaxValue}]") + s"does not fit in range [${Long.MinValue}, ${Long.MaxValue}]") // Double Literal assertEqual("10.0D", Literal(10.0D)) + intercept("-1.8E308D", s"does not fit in range") + intercept("1.8E308D", s"does not fit in range") // TODO we need to figure out if we should throw an exception here! assertEqual("1E309", Literal(Double.PositiveInfinity)) }