Support ON UPDATE CURRENT_TIMESTAMP in MySQL #1967
Conversation
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1967 +/- ##
=========================================
Coverage 89.27% 89.27%
- Complexity 8049 8053 +4
=========================================
Files 232 232
Lines 24519 24526 +7
=========================================
+ Hits 21889 21896 +7
Misses 2630 2630
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| @@ -264,6 +264,8 @@ public function getColumnFromRow(array $row, Table $table): Column | |||
| // BLOBs can't have any default values in MySQL | |||
| $default = preg_match('~blob|text~', $nativeType) ? null : $row['Default']; | |||
|
|
|||
| $extra = $row['Extra']; | |||
There was a problem hiding this comment.
Can you explain why this is necessary here? When I look at the code, it does not look like $row is changed between here and line 300, where you use it.
There was a problem hiding this comment.
The idea was to take it out of the row just once in case it is used multiple times so that there is only one index access for Extra. But it turned out to be used only once, so I could move it to line 300 as well. However, it might be easier to read the way it is now. Please let me know what you prefer and I'll adjust it accordingly.
There was a problem hiding this comment.
Makes sense. Personally, I'd prefer it in line 300, as there is no benefit to moving it further up at the moment.
There was a problem hiding this comment.
Sounds good to me. I changed it accordingly.
|
@Mau04 It looks like the old maintainer of Propel2, @dereuromark, has left the project, and it looks like there is no successor at Spryker at the moment. I have contacted the owner, and he told me that the project will keep being maintained by Spryker, but I haven't heard back if/when there will be someone who actively maintains Propel2, i.e. merges this PRs. |
|
Hello @mringler & @Mau04 Thank you for your vital role in this community. |
There was a problem hiding this comment.
Hi @Mau04!
Thanks for contributing!
Would be great to make CI green and the PR will be merged afterwards.
No other objections from our side.
This PR adds support for
defaultExpr="CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP"for MySQL as requested in #1203. Previously, a migration would always contain an ALTER Statement to setON UPDATE CURRENT_TIMESTAMPalthough it is already like that in the database. The root cause was that theExtrapart was not checked in theMysqlSchemaParserclass.