/framework

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

[5.7] Add computed support to SQL Server schema grammar #27346

Merged
merged 4 commits into from Jan 30, 2019
+79 −2

Conversation

Reviewers

@staudenmeir staudenmeir

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@jlsjonas @GrahamCampbell @staudenmeir @taylorotwell
@jlsjonas
Copy link
Contributor

jlsjonas commented Jan 29, 2019

SQL Server also supports computed columns (Azure & 2016+), but as a type; not a modifier.

inspired by #13430 but for SQL Server
Sadly enough SQL Server defines this instead of a data type, so I couldn't re-use the existing methods.

Instead, a "virtual" type was introduced, with a "persisted" modifier; reflecting the mysql-only virtualAs & storedAs modifiers respectively.

@GrahamCampbell GrahamCampbell closed this Jan 29, 2019

@GrahamCampbell GrahamCampbell reopened this Jan 29, 2019

@GrahamCampbell GrahamCampbell changed the title Add computed support to SQL Server schema grammar [5.7] Add computed support to SQL Server schema grammar Jan 29, 2019

@GrahamCampbell

This comment has been minimized.

Sign in to view
Copy link
Member

GrahamCampbell commented Jan 29, 2019

Sorry, wrong button!
@jlsjonas

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

jlsjonas commented Jan 29, 2019

Tests should actually work out now 😅 🤞
@staudenmeir

This comment has been minimized.

Sign in to view
Copy link
Contributor

staudenmeir commented Jan 29, 2019 •
edited

IMO, virtual() is not an ideal method name. If you add the ->persisted() modifier, you create a "virtual" column that isn't actually virtual.

I would prefer generated() (used for MySQL's virtualAs() and storedAs()) or computed() (used in SQL Server documentation).
@staudenmeir

staudenmeir reviewed Jan 29, 2019
View changes

Show resolved Hide resolved src/Illuminate/Database/Schema/Blueprint.php Outdated
@staudenmeir

staudenmeir reviewed Jan 29, 2019
View changes

Show resolved Hide resolved src/Illuminate/Database/Schema/Grammars/SqlServerGrammar.php Outdated
@jlsjonas

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

jlsjonas commented Jan 30, 2019

(cleaned up commits)

@jlsjonas jlsjonas force-pushed the jlsjonas:patch-2 branch from 72641c0 to 5443dca Jan 30, 2019

@staudenmeir

This comment has been minimized.

Sign in to view
Copy link
Contributor

staudenmeir commented Jan 30, 2019

Excellent, MySqlGrammar::typeComputed() is a great addition.

@taylorotwell taylorotwell merged commit 5443dca into laravel:5.7 Jan 30, 2019
2 checks passed

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment