Skip to content

[SPARK-29983][SQL] Add an independent config for an optional INTERVAL clause #26623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
maropu wants to merge 5 commits into from
Closed

[SPARK-29983][SQL] Add an independent config for an optional INTERVAL clause #26623

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5e20371
Fix
maropu Nov 21, 2019
6d0223e
Fix
maropu Nov 21, 2019
d53ec87
Fix
maropu Nov 21, 2019
5868ef1
Fix
maropu Nov 21, 2019
7e63c0f
Fix
maropu Nov 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ grammar SqlBase;
* When true, the behavior of keywords follows ANSI SQL standard.
*/
public boolean SQL_standard_keyword_behavior = false;

/**
* When true, an INTERVAL keyword can be optional for interval values in SQL statements.
*/
public boolean optional_interval = false;
}

singleStatement
Expand Down Expand Up @@ -788,7 +793,7 @@ booleanValue

interval
: INTERVAL (errorCapturingMultiUnitsInterval | errorCapturingUnitToUnitInterval)?
| {SQL_standard_keyword_behavior}? (errorCapturingMultiUnitsInterval | errorCapturingUnitToUnitInterval)
| {optional_interval}? (errorCapturingMultiUnitsInterval | errorCapturingUnitToUnitInterval)
;

errorCapturingMultiUnitsInterval
Expand Down
2 changes: 2 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseDriver.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ abstract class AbstractSqlParser(conf: SQLConf) extends ParserInterface with Log
lexer.addErrorListener(ParseErrorListener)
lexer.legacy_setops_precedence_enbled = conf.setOpsPrecedenceEnforced
lexer.SQL_standard_keyword_behavior = SQLStandardKeywordBehavior
lexer.optional_interval = conf.optionalInterval
Copy link
Contributor

@cloud-fan cloud-fan Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be dialect == Spark && ansi == true && optionalInterval

For pgsql diaelct, INTERVAL can't be optional. for non-ansi mode, year/month/day... are not reserved keywords and we can't ommit INTERVAL prefix anyway.

Copy link
Member Author

@maropu maropu Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok


val tokenStream = new CommonTokenStream(lexer)
val parser = new SqlBaseParser(tokenStream)
Expand All @@ -109,6 +110,7 @@ abstract class AbstractSqlParser(conf: SQLConf) extends ParserInterface with Log
parser.addErrorListener(ParseErrorListener)
parser.legacy_setops_precedence_enbled = conf.setOpsPrecedenceEnforced
parser.SQL_standard_keyword_behavior = SQLStandardKeywordBehavior
parser.optional_interval = conf.optionalInterval

try {
try {
Expand Down
9 changes: 9 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1787,6 +1787,13 @@ object SQLConf {
val SQL_STANDARD, ISO_8601, MULTI_UNITS = Value
}

val OPTIONAL_INTERVAL =
buildConf("spark.sql.optionalInterval")
Copy link
Contributor

@cloud-fan cloud-fan Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's put it under the spark.sql.parser namespace

Copy link
Member Author

@maropu maropu Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

.doc("When true, an INTERVAL keyword can be optional for interval values in SQL statements," +
" e.g., 'SELECT 1 day'.")
.booleanConf
.createWithDefault(false)

val INTERVAL_STYLE = buildConf("spark.sql.intervalOutputStyle")
.doc("When converting interval values to strings (i.e. for display), this config decides the" +
" interval string format. The value SQL_STANDARD will produce output matching SQL standard" +
Expand Down Expand Up @@ -2513,6 +2520,8 @@ class SQLConf extends Serializable with Logging {
def storeAssignmentPolicy: StoreAssignmentPolicy.Value =
StoreAssignmentPolicy.withName(getConf(STORE_ASSIGNMENT_POLICY))

def optionalInterval: Boolean = getConf(OPTIONAL_INTERVAL)

def intervalOutputStyle: IntervalStyle.Value = IntervalStyle.withName(getConf(INTERVAL_STYLE))

def dialect: Dialect.Value = Dialect.withName(getConf(DIALECT))
Expand Down
1 change: 1 addition & 0 deletions sql/core/src/test/resources/sql-tests/inputs/ansi/interval.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
--SET spark.sql.optionalInterval=true
--IMPORT interval.sql

-- the `interval` keyword can be omitted with ansi mode
Expand Down