Support column names in more places

[benesch]

See individual commits for details.

[coveralls]

Pull Request Test Coverage Report for Build 251

• 60 of 64 (93.75%) changed or added relevant lines in 4 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 90.127%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/sqlparser.rs	11	13	84.62%
tests/sqlparser_common.rs	30	32	93.75%

Totals
Change from base Build 234:	0.0%
Covered Lines:	3396
Relevant Lines:	3768

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 251

• 60 of 64 (93.75%) changed or added relevant lines in 4 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 90.127%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/sqlparser.rs	11	13	84.62%
tests/sqlparser_common.rs	30	32	93.75%

Totals
Change from base Build 234:	0.0%
Covered Lines:	3396
Relevant Lines:	3768

---

💛 - Coveralls

[nickolay]

LGTM! I'm inclined to rename Cte::renamed_columns to columns and move it before the query to match the style of this PR once you finish landing your patches.

I'm always unsure if I want to introduce a new struct vs tolerate some duplication in existing structs, so if you have a couple of words to say about why you chose to extract TableAlias, I'd appreciate it. Note that I don't have anything against it, so just merging is fine as well.

(Ouch, this probably conflicts with the recently landed "WITH options" PR -- the expected order in CREATE VIEW is view_name (columns, list) WITH (options) AS I think? Having dealt with merge conflicts a lot, I feel your pain - thanks for doing this again! and if I can make this easier, please let me know.)

[benesch]

I'm always unsure if I want to introduce a new struct vs tolerate some duplication in existing structs, so if you have a couple of words to say about why you chose to extract TableAlias, I'd appreciate it. Note that I don't have anything against it, so just merging is fine as well.

Sure thing! I thought having a separate struct that could be wrapped in an Option better communicated the set of possibilities. That is, sticking both a table_alias and a column_aliases field in the outer struct, like

struct Table {
    table_alias: Option<SQLObjectName>,
    column_aliases: Vec<SQLIdentifier>
}

indicates that it might be valid to have column aliases without a table alias, when it is not in fact valid. (You can specify a table alias without specifying column aliases, but you cannot specify a column alias without specifying a table alias.)

(Ouch, this probably conflicts with the recently landed "WITH options" PR -- the expected order in CREATE VIEW is view_name (columns, list) WITH (options) AS I think? Having dealt with merge conflicts a lot, I feel your pain - thanks for doing this again! and if I can make this easier, please let me know.)

No problem! The rebase is in, and the conflicts weren't too bad. I really appreciate the speed with which you're moving through these PRs, and I'm happy to do a bit of work to split and reintegrate these changes to make the review easier. In other words, it's the overall rate of divergence that's scary and time-consuming, not the latency of any individual review, and the rate of divergence is very solidly negative at the moment! :)

[nickolay]

You can specify a table alias without specifying column aliases, but you cannot specify a column alias without specifying a table alias.

Ah, I have not thought of this. Thanks for clarifying - this makes perfect sense to me.