Shorten index name of calendar changes table#12447
Conversation
nickvergessen
added
bug
3. to review
|
Tests running in https://travis-ci.org/nextcloud/notifications/builds/454880005 |
nickvergessen
force-pushed
the
bugfix/12446/fix-oracle-install-again
branch
from
36365da to
38752aa
Compare
| public function setApcuPrefix($apcuPrefix) | ||
| { | ||
| $this->apcuPrefix = function_exists('apcu_fetch') && filter_var(ini_get('apc.enabled'), FILTER_VALIDATE_BOOLEAN) ? $apcuPrefix : null; | ||
| $this->apcuPrefix = function_exists('apcu_fetch') && ini_get('apc.enabled') ? $apcuPrefix : null; |
There was a problem hiding this comment.
This looks like you are using an outdated composer. Because my local composer (which says is up to date) also produced the output as stated in the deleted line and the checker CI job also complains.
There was a problem hiding this comment.
yeah, still had 1.7.2, now all other ClassLoaders where updated to the filter_var version
| /** @var ISchemaWrapper $schema */ | ||
| $schema = $schemaClosure(); | ||
|
|
||
| if ($schema->hasTable('calendarchanges')) { |
There was a problem hiding this comment.
Why is this needed? Can't we assume, that it is there? Same for the other two if statements below.
There was a problem hiding this comment.
I just copied the original migration, yes we could assume it exists, but does it hurt?
There was a problem hiding this comment.
Not really - it just confuses.
Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
bugfix/12446/fix-oracle-install-again
branch
from
38752aa to
aa88254
Compare
|
I guess it's not wrong to use short table/index/whatever names but oracle 11 is eol (without a subscription) since 2017. The 30 characters limitation has been increased to 128 with 12.2 (https://blog.dbi-services.com/oracle-12cr2-long-identifiers/). Maybe running the tests with a newer oracle db would work? 😕 |
|
Well most people that use Oracle in a larger instance have a subscription. |
|
@rullzer Mind to review? |
4 participants
Fix #12446
Fix #12410