Skip to content
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

airframe-sql: Remove unnecessary condition from AllColumns resolution #3061

Merged
merged 1 commit into from
Jul 13, 2023
Merged

airframe-sql: Remove unnecessary condition from AllColumns resolution #3061

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion airframe-sql/src/main/scala/wvlet/airframe/sql/analyzer/TypeResolver.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ object TypeResolver extends LogSupport {
case other => toResolvedAttribute(other.name, other)
}
List(a.copy(columns = Some((qualifier match {
case Some(q) => allColumns.filter(c => c.qualifier.contains(q) || c.tableAlias.contains(q))
case Some(q) => allColumns.filter(_.tableAlias.contains(q))
Copy link
Member

Choose a reason for hiding this comment

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

Good. This is because AllColumn.outputAttributes have both the qualifier and tableAlias

Copy link
Member Author

@takezoe takezoe Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is because AllColumn.outputAttributes have both the qualifier and tableAlias

This is not accurate explanation.

Without this fix, we needed to check qualifiers for the following case but now this case is covered by tableAlias.

select t1.* from (select t1.time from A) t1

On the other hand, checking qualifiers allows to resolve cases like below, but actually these are not valid SQL which means the current logic can resolve even invalid cases.

select A.* from (select A.time from A)
select t1.* from (select t1.time from A t1)

case None => allColumns
}).map(_.withQualifier(None)))))
case _ =>
Expand Down