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

Get view definition for SQL Server #4844

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

kitloong
Copy link
Contributor

@kitloong kitloong commented Oct 2, 2021

Q A
Type improvement
BC Break no
Fixed issues -

Summary

Currently, SQLServer2012Platform::getListViewsSQL does not export view definition.
This PR updated the SQL so that Doctrine\DBAL\Schema\View::getSql() will return view definition instead of empty string.

Copy link
Member

@morozov morozov left a comment

Choose a reason for hiding this comment

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

Thank you for the patch. Please add an integration test or reuse an existing one.

@morozov
Copy link
Member

morozov commented Oct 7, 2021

Given that the DBAL doesn't use views itself, I'd rather classify this as an improvement. In this case, the patch should target 3.2.x.

@kitloong kitloong changed the base branch from 3.1.x to 3.2.x October 8, 2021 04:57
@kitloong
Copy link
Contributor Author

kitloong commented Oct 8, 2021

Hi @morozov , thank you for review.

I will add tests for this.

I am sorry, should I change the current PR base to 3.2.x? Or checkout a new branch from 3.2.x to commit the changes?

@morozov
Copy link
Member

morozov commented Oct 8, 2021

[…] should I change the current PR base to 3.2.x?

Yes, this was the first step. Now you need to rebase your changes on 3.2.x and force-push the changes:

$ git checkout fix-sqlserver-view
$ git rebase HEAD~ --onto 3.2.x # this assumes you want to rebase one commit
$ git push -f # adjust according to your remote naming

@kitloong
Copy link
Contributor Author

Hello @morozov

Following items completed.

  • Code update based on last reviewed.
  • Rebase 3.2.x.
  • Update integration test.

Please review.

@morozov
Copy link
Member

morozov commented Oct 13, 2021

@kitloong looks good. Please squash again.

Currently, SQLServer2012Platform::getListViewsSQL does not export view definition.
This commit get view definition for SQL server.
Reference: https://docs.microsoft.com/en-us/sql/relational-databases/views/get-information-about-a-view?view=sql-server-ver15#to-get-the-definition-and-properties-of-a-view
@morozov morozov added this to the 3.2.0 milestone Oct 14, 2021
@morozov morozov merged commit fd27592 into doctrine:3.2.x Oct 14, 2021
@morozov
Copy link
Member

morozov commented Oct 14, 2021

Thanks, @kitloong!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2022
@kitloong kitloong deleted the fix-sqlserver-view branch October 17, 2022 08:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants