-
Notifications
You must be signed in to change notification settings - Fork 73
DOC Document generated column support in the ORM #775
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
DOC Document generated column support in the ORM #775
Conversation
5166210 to
2b724a2
Compare
|
It also seems the index support is only available from MySQL 8, should we mention that? |
The earliest supported version of MySQL is 8.0 as of CMS 6.0.0 so I don't think that needs to be mentioned. |
2b724a2 to
271b222
Compare
michalkleiner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads well
en/08_Changelogs/6.1.0.md
Outdated
|
|
||
| - sorting in a gridfield on a complex summary field: It's common to use a getter method to get some value derived from your database fields (e.g. a discounted price), and use that in `summary_fields`. With a generated column, you can remove the getter method and the [`GridFieldSortableHeader`](api:SilverStripe\Forms\GridField\GridFieldSortableHeader) component will be able to sort using the column. | ||
| - using functional indexes: generated columns can be included in indexes, allowing you to sort or filter by complex expressions in an efficient way. | ||
| - data integrity: unlike generating values inside an `onBeforeWrite()` extension hook, generated column values will be correct even if you update the record with raw SQL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - data integrity: unlike generating values inside an `onBeforeWrite()` extension hook, generated column values will be correct even if you update the record with raw SQL. | |
| - data integrity: unlike generating values inside an `onBeforeWrite()` extension hook, generated column values will be correct even if you update the record with raw SQL. |
Probably also restoring old versions of records and archived records which have had columns added since the old record was created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put the diff in a diff checker and there's no change in your suggested change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opps
Add something like this (feel free to alter)
This also ensures restored and archived records, with may have new columns since the record was created, remain accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| - sorting in a gridfield on a complex summary field: It's common to use a getter method to get some value derived from your database fields (e.g. a discounted price), and use that in `summary_fields`. With a generated column, you can remove the getter method and the [`GridFieldSortableHeader`](api:SilverStripe\Forms\GridField\GridFieldSortableHeader) component will be able to sort using the column. | ||
| - using functional indexes: generated columns can be included in indexes, allowing you to sort or filter by complex expressions in an efficient way. | ||
| - data integrity: unlike generating values inside an `onBeforeWrite()` extension hook, generated column values will be correct even if you update the record with raw SQL. | ||
| - reduce repetition: instead of using a complex expression in multiple different places, you can just give the expression a name with a generated column. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this means?
Normally you'd just have a getMyValue() method on a model? i.e. logic in a single place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have multiple SQL queries that use the same complex expression, you can replace those complex expressions with a single named generated column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not really registering with me
What would an example of this be using the ORM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I have some record.
- I want to filter by an expression in a template, so I add a controller method that does a custom SQL query that uses that expression. Even something simple like
CONCAT("FieldOne", "FieldTwo"). - In another area of the code that's completely separate from the above controller, I am using the same
DataObjectmodel. I want to filter or sort by that same expression.
Instead of having CONCAT("FieldOne", "FieldTwo") in both places, I can just add a generated column named FieldConcat and use that field name instead.
en/08_Changelogs/6.1.0.md
Outdated
|
|
||
| Generated column support has been added to the MySQL database connector in `silverstripe/framework`, but is not guaranteed to work for other database servers. | ||
|
|
||
| If you maintain a module that adds support for another database server, you'll need to implement the new [`DBSchemaManager::makeGenerated()`](SilverStripe\ORM\Connect\DBSchemaManager::makeGenerated()) and [`DBSchemaManager::needRebuildColumn()`](SilverStripe\ORM\Connect\DBSchemaManager::needRebuildColumn()) methods. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| If you maintain a module that adds support for another database server, you'll need to implement the new [`DBSchemaManager::makeGenerated()`](SilverStripe\ORM\Connect\DBSchemaManager::makeGenerated()) and [`DBSchemaManager::needRebuildColumn()`](SilverStripe\ORM\Connect\DBSchemaManager::needRebuildColumn()) methods. | |
| If you maintain a module that adds support for another database server, you'll need to implement the new [`DBSchemaManager::makeGenerated()`](SilverStripe\ORM\Connect\DBSchemaManager::makeGenerated()) and [`DBSchemaManager::needRebuildColumn()`](SilverStripe\ORM\Connect\DBSchemaManager::needRebuildColumn()) methods to support generated columns. |
Presumbably you don't HAVE to update the the module, because there's no BC changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You do HAVE to update the module to support generated columns if you want to support generated columns lol. I think "to support generated columns" is very heavily implied but I'll add it to remove any hint of confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
271b222 to
261d34d
Compare
en/08_Changelogs/6.1.0.md
Outdated
|
|
||
| - sorting in a gridfield on a complex summary field: It's common to use a getter method to get some value derived from your database fields (e.g. a discounted price), and use that in `summary_fields`. With a generated column, you can remove the getter method and the [`GridFieldSortableHeader`](api:SilverStripe\Forms\GridField\GridFieldSortableHeader) component will be able to sort using the column. | ||
| - using functional indexes: generated columns can be included in indexes, allowing you to sort or filter by complex expressions in an efficient way. | ||
| - data integrity: unlike generating values inside an `onBeforeWrite()` extension hook, generated column values will be correct even if you update the record with raw SQL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opps
Add something like this (feel free to alter)
This also ensures restored and archived records, with may have new columns since the record was created, remain accurate.
| - sorting in a gridfield on a complex summary field: It's common to use a getter method to get some value derived from your database fields (e.g. a discounted price), and use that in `summary_fields`. With a generated column, you can remove the getter method and the [`GridFieldSortableHeader`](api:SilverStripe\Forms\GridField\GridFieldSortableHeader) component will be able to sort using the column. | ||
| - using functional indexes: generated columns can be included in indexes, allowing you to sort or filter by complex expressions in an efficient way. | ||
| - data integrity: unlike generating values inside an `onBeforeWrite()` extension hook, generated column values will be correct even if you update the record with raw SQL. | ||
| - reduce repetition: instead of using a complex expression in multiple different places, you can just give the expression a name with a generated column. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not really registering with me
What would an example of this be using the ORM?
| // Discount the car for a fixed percentage based on the condition of the car | ||
| // phpcs:ignore Generic.Files.LineLength.TooLong | ||
| 'Discount' => 'Generated("Percentage", "CASE WHEN (\\"Condition\\"=\'Junk\') THEN 0.75 WHEN (\\"Condition\\"=\'Fair\') THEN 0.25 ELSE 0 END", "STORED")', | ||
| 'DiscountPrice' => 'Generated("Currency", "\\"Price\\" * (1.0 - \\"Discount\\")", "VIRTUAL")', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably makes sense to use a heredoc for Discount and keep as a one liner for DiscountPrice to show both ways - see silverstripe/silverstripe-framework#11774 (comment) for example of the heredoc syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Used a nowdoc because it's more appropriate here - we don't want any string interpolation.
261d34d to
43fe9dd
Compare
| 'Condition' => 'Enum("New,Fair,Junk", "Fair")', | ||
| 'Price' => 'Currency', | ||
| // Discount the car for a fixed percentage based on the condition of the car | ||
| 'Discount' => <<<'SPEC' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See silverstripe/silverstripe-assets#694 (comment) for why I used SPEC as the delimiter.
We're skipping some best practice e.g. setting the table_name for the sake of brevity.
|
|
||
| class Player extends DataObject | ||
| { | ||
| // ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these to indicate we're skipping some best practices (e.g. table_name) for the sake of brevity.
* DOC Document generated column support in the ORM * DOC Add ellipses to indicate skipping bits for brevity We're skipping some best practice e.g. setting the table_name for the sake of brevity.
Issue