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

Make sure blob columns are correctly converted as parameters #4071

Merged
merged 2 commits into from
Apr 4, 2017

Conversation

nickvergessen
Copy link
Member

Fix #173
Fix #3536

Signed-off-by: Joas Schilling <coding@schilljs.com>
@mention-bot
Copy link

@nickvergessen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bantu, @bartv2 and @MorrisJobke to be potential reviewers.

@nickvergessen
Copy link
Member Author

I'd like to backport this to 11, so we can tell people that they can migrate away from sqlite on 11+ at least, @karlitschek

@karlitschek
Copy link
Member

nice. please backport 👍

@codecov-io
Copy link

Codecov Report

Merging #4071 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #4071      +/-   ##
============================================
- Coverage     54.24%   54.23%   -0.01%     
- Complexity    21285    21292       +7     
============================================
  Files          1310     1310              
  Lines         81187    81199      +12     
  Branches       1284     1284              
============================================
+ Hits          44036    44037       +1     
- Misses        37151    37162      +11
Impacted Files Coverage Δ Complexity Δ
core/Command/Db/ConvertType.php 0% <0%> (ø) 51 <7> (+7) ⬆️
apps/files_external/lib/Lib/Storage/SMB.php 47.22% <0%> (+0.39%) 112% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec6853a...2961c73. Read the comment docs.

}

$prefix = $this->config->getSystemValue('dbtableprefix', 'oc_');
$this->columnTypes[$table][$column] = null;
Copy link
Member

Choose a reason for hiding this comment

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

setting this makes no difference, isset will also return false if the value is null.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I used a different value before...

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Signed-off-by: Joas Schilling <coding@schilljs.com>
@rullzer
Copy link
Member

rullzer commented Mar 30, 2017

Is there anyway to have tests for this?

@nickvergessen
Copy link
Member Author

The problem is, I can create a test whether the isset($data[$table][$colum]) is true, not sure it if makes sense. We also need to maintain the list manually...

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

ok lets do this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants