Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ singleTableSchema

statement
: query #statementDefault
| insertStatement #insertStatementDefault
| multiSelectStatement #multiSelectStatementDefault
| ctes? dmlStatementNoWith #dmlStatement
| USE db=identifier #use
| CREATE database (IF NOT EXISTS)? identifier
(COMMENT comment=STRING)? locationSpec?
Expand Down Expand Up @@ -356,14 +355,14 @@ resource
: identifier STRING
;

insertStatement
: (ctes)? insertInto queryTerm queryOrganization #singleInsertQuery
| (ctes)? fromClause multiInsertQueryBody+ #multiInsertQuery
Copy link
Contributor Author

Choose a reason for hiding this comment

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

move the ctes? to the parent entry, to save duplicated code. This is a simplification of #24150

dmlStatementNoWith
: insertInto queryTerm queryOrganization #singleInsertQuery
| fromClause multiInsertQueryBody+ #multiInsertQuery
;

queryNoWith
: queryTerm queryOrganization #noWithQuery
| fromClause selectStatement #queryWithFrom
| fromClause selectStatement+ #queryWithFrom
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we name the label #queriesWithFrom ? I am fine if we stay as is :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, multi-select is still one query :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@cloud-fan Okay.. sounds good to me.

;

queryOrganization
Expand All @@ -379,10 +378,6 @@ multiInsertQueryBody
: insertInto selectStatement
;

multiSelectStatement
: (ctes)? fromClause selectStatement+ #multiSelect
;

selectStatement
: querySpecification queryOrganization
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
query.optionalMap(ctx.ctes)(withCTE)
}

override def visitDmlStatement(ctx: DmlStatementContext): AnyRef = withOrigin(ctx) {
val dmlStmt = plan(ctx.dmlStatementNoWith)
// Apply CTEs
dmlStmt.optionalMap(ctx.ctes)(withCTE)
}

private def withCTE(ctx: CtesContext, plan: LogicalPlan): LogicalPlan = {
val ctes = ctx.namedQuery.asScala.map { nCtx =>
val namedQuery = visitNamedQuery(nCtx)
Expand All @@ -129,11 +135,21 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging

override def visitQueryWithFrom(ctx: QueryWithFromContext): LogicalPlan = withOrigin(ctx) {
val from = visitFromClause(ctx.fromClause)
validate(ctx.selectStatement.querySpecification.fromClause == null,
"Individual select statement can not have FROM cause as its already specified in the" +
" outer query block", ctx)
withQuerySpecification(ctx.selectStatement.querySpecification, from).
optionalMap(ctx.selectStatement.queryOrganization)(withQueryResultClauses)
val selects = ctx.selectStatement.asScala.map { select =>
validate(select.querySpecification.fromClause == null,
"This select statement can not have FROM cause as its already specified upfront",
select)

withQuerySpecification(select.querySpecification, from).
// Add organization statements.
optionalMap(select.queryOrganization)(withQueryResultClauses)
}
// If there are multiple SELECT just UNION them together into one query.
if (selects.length == 1) {
selects.head
} else {
Union(selects)
}
}

override def visitNoWithQuery(ctx: NoWithQueryContext): LogicalPlan = withOrigin(ctx) {
Expand Down Expand Up @@ -182,47 +198,21 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}

// If there are multiple INSERTS just UNION them together into one query.
val insertPlan = inserts match {
case Seq(query) => query
case queries => Union(queries)
}
// Apply CTEs
insertPlan.optionalMap(ctx.ctes)(withCTE)
}

override def visitMultiSelect(ctx: MultiSelectContext): LogicalPlan = withOrigin(ctx) {
val from = visitFromClause(ctx.fromClause)

// Build the insert clauses.
val selects = ctx.selectStatement.asScala.map {
body =>
validate(body.querySpecification.fromClause == null,
"Multi-select queries cannot have a FROM clause in their individual SELECT statements",
body)

withQuerySpecification(body.querySpecification, from).
// Add organization statements.
optionalMap(body.queryOrganization)(withQueryResultClauses)
}

// If there are multiple INSERTS just UNION them together into one query.
val selectUnionPlan = selects match {
case Seq(query) => query
case queries => Union(queries)
if (inserts.length == 1) {
inserts.head
} else {
Union(inserts)
}
// Apply CTEs
selectUnionPlan.optionalMap(ctx.ctes)(withCTE)
}

/**
* Create a logical plan for a regular (single-insert) query.
*/
override def visitSingleInsertQuery(
ctx: SingleInsertQueryContext): LogicalPlan = withOrigin(ctx) {
val insertPlan = withInsertInto(ctx.insertInto(),
plan(ctx.queryTerm).optionalMap(ctx.queryOrganization)(withQueryResultClauses))
// Apply CTEs
insertPlan.optionalMap(ctx.ctes)(withCTE)
withInsertInto(
ctx.insertInto(),
plan(ctx.queryTerm).optionalMap(ctx.queryOrganization)(withQueryResultClauses))
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,19 @@ class PlanParserSuite extends AnalysisTest {
table("a").select(star()).union(table("a").where('s < 10).select(star())))
intercept(
"from a select * select * from x where a.s < 10",
"Multi-select queries cannot have a FROM clause in their individual SELECT statements")
"This select statement can not have FROM cause as its already specified upfront")
Copy link
Member

Choose a reason for hiding this comment

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

you know what? cannot is correct too and more usual :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

it's fine. I didn't mean we should fix now. just wanted to say it :-).

intercept(
"from a select * from b",
"Individual select statement can not have FROM cause as its already specified in " +
"the outer query block")
"This select statement can not have FROM cause as its already specified upfront")
assertEqual(
"from a insert into tbl1 select * insert into tbl2 select * where s < 10",
table("a").select(star()).insertInto("tbl1").union(
table("a").where('s < 10).select(star()).insertInto("tbl2")))
assertEqual(
"select * from (from a select * select *)",
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for adding this.

table("a").select(star())
.union(table("a").select(star()))
.as("__auto_generated_subquery_name").select(star()))
}

test("query organization") {
Expand Down