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.

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

Fix incorrect behavior when preparing SELECT * preceded by a WITH #1179

Merged
merged 2 commits into from
Dec 28, 2022

Conversation

marmphco
Copy link
Contributor

In #1139 I introduced support for the WITH clause. My implementation contains a bug: preparing a query containing a SELECT * preceded by a WITH causes an error to be thrown.

Consider the following statement:

WITH temp AS ( SELECT id, email from users) SELECT * from temp

An error is thrown when preparing this statement because the glob expansion procedure tries to determine the column names for the result by looking up the column names for the query SELECT * from temp. This does not work because temp is a temporary view defined in the WITH clause.

To fix this, I modified the glob expansion procedure to include the WITH clause in the query used to look up the result column names.

In  stephencelis#1139 I introduced support for the `WITH` clause. My implementation contains a bug: the statement preparer doesn't produce the correct result column names for queries containing a `SELECT *` preceded by a `WITH`.

For example, consider the following statement:

```
WITH temp AS ( SELECT id, email from users) SELECT * from temp
```

An error would be thrown when preparing this statement because the glob expansion procedure would try to look up the column names for the result by looking up the column names for the query `SELECT * from temp`. This does not work because `temp` is a temporary view defined in the `WITH` clause.

To fix this, I modified the glob expansion procedure to include the `WITH` clause in the query used to look up the result column names.
func expandGlob(_ namespace: Bool) -> (QueryType) throws -> Void {
{ (queryType: QueryType) throws -> Void in
var query = type(of: queryType).init(queryType.clauses.from.name, database: queryType.clauses.from.database)
query.clauses.select = queryType.clauses.select
query.clauses.with = strip(queryType.clauses.with)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The strip function feels a little excessive. Doing this instead also works:

query.clauses.with = queryType.clauses.with

but the query will include things that aren't necessary for determining the result column names, like filters, sorting, and grouping.

@marmphco marmphco marked this pull request as ready for review December 23, 2022 09:46
@nathanfallet nathanfallet merged commit 02d69a2 into stephencelis:master Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants