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

Re-introduction of GeneratePseudoFromClause for specific "empty" FROM clauses #18999

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Re-introduction of GeneratePseudoFromClause for specific "empty" FROM clauses #18999

merged 1 commit into from
Nov 26, 2019

Conversation

cincuranet
Copy link
Contributor

@cincuranet cincuranet commented Nov 20, 2019

For #18926.

It would really really really help [1] if this would be considered for merge 🙏 and before 3.1 ships 🤞. I don't see any breaking changes here, because in 3.0 this was non-existing and hence nobody could override, hence nothing is/was/will be broken.

[1]: It would help because copying VisitSelect and duplicating all the code is impractical because of some private methods usage and also makes no sense when simple plug in point can handle it.

@cincuranet cincuranet marked this pull request as ready for review November 20, 2019 20:52
@smitpatel smitpatel requested a review from ajcvickers November 20, 2019 23:08
@smitpatel
Copy link
Contributor

This will be done in master as part of #18923

@cincuranet
Copy link
Contributor Author

cincuranet commented Nov 21, 2019 via email

@ajcvickers
Copy link
Contributor

@cincuranet 3.1 is already finalized, so the change cannot go there. Also, we are not allowed to make API changes in patch releases, except for very exceptional circumstances--i.e. security. So, unfortunately this means that the next release this change can be in is 5.0.

@cincuranet
Copy link
Contributor Author

cincuranet commented Nov 25, 2019 via email

@ajcvickers ajcvickers requested a review from smitpatel November 25, 2019 17:15
@ajcvickers ajcvickers merged commit 67b36d3 into dotnet:master Nov 26, 2019
@ajcvickers
Copy link
Contributor

@cincuranet Thanks!

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.

3 participants