Skip to content

Commit

Permalink
MDL-33172 - filestorage: Fix oracle incompatibilites
Browse files Browse the repository at this point in the history 
Fixes the following problems:

* When we select from two tables with the same named fields (id)
  and ask to sort by that field, Oracle doesn't know which table to
  sort frm unless that field is named in the SELECT. Here we do that
  by explicitly naming the fields. This keeps compatibility with before
  the reference table was added.

* Text comparisong without sql_compare_text
  • Loading branch information
@danpoltawski
danpoltawski authored and Sam Hemelryk committed May 24, 2012
1 parent ac29403 commit 3447100
Showing 1 changed file with 50 additions and 14 deletions.
64 changes: 50 additions & 14 deletions lib/filestorage/file_storage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ protected function create_imagefile_preview(stored_file $file, $mode) {
public function get_file_by_id($fileid) {
global $DB;

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand All @@ -293,7 +293,7 @@ public function get_file_by_id($fileid) {
public function get_file_by_hash($pathnamehash) {
global $DB;

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand Down Expand Up @@ -370,7 +370,7 @@ public function is_area_empty($contextid, $component, $filearea, $itemid = false
*/
public function get_external_files($repositoryid, $sort = 'sortorder, itemid, filepath, filename') {
global $DB;
$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand Down Expand Up @@ -407,7 +407,7 @@ public function get_area_files($contextid, $component, $filearea, $itemid = fals
$itemidsql = '';
}

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand Down Expand Up @@ -504,7 +504,7 @@ public function get_directory_files($contextid, $component, $filearea, $itemid,
$dirs = $includedirs ? "" : "AND filename <> '.'";
$length = textlib::strlen($filepath);

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand Down Expand Up @@ -534,7 +534,7 @@ public function get_directory_files($contextid, $component, $filearea, $itemid,
$length = textlib::strlen($filepath);

if ($includedirs) {
$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand All @@ -553,7 +553,7 @@ public function get_directory_files($contextid, $component, $filearea, $itemid,
}
}

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand Down Expand Up @@ -772,7 +772,7 @@ public function create_file_from_storedfile($filerecord, $fileorid) {
unset($filerecord['contenthash']);
unset($filerecord['pathnamehash']);

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
Expand Down Expand Up @@ -1669,11 +1669,12 @@ public static function unpack_reference($str) {
*/
public function search_references($str) {
global $DB;
$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
WHERE ".$DB->sql_compare_text('r.reference').' = '.$DB->sql_compare_text('?')."
AND (f.component <> ? OR f.filearea <> ?)";

$rs = $DB->get_recordset_sql($sql, array($str, 'user', 'draft'));
$files = array();
Expand All @@ -1699,7 +1700,8 @@ public function search_references_count($str) {
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
WHERE ".$DB->sql_compare_text('r.reference').' = '.$DB->sql_compare_text('?')."
AND (f.component <> ? OR f.filearea <> ?)";

$count = $DB->count_records_sql($sql, array($str, 'user', 'draft'));
return $count;
Expand All @@ -1726,11 +1728,12 @@ public function get_references_by_storedfile($storedfile) {

$reference = self::pack_reference($params);

$sql = "SELECT f.*, r.repositoryid, r.reference
$sql = "SELECT ".self::instance_sql_fields('f', 'r')."
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
WHERE ".$DB->sql_compare_text('r.reference').' = '.$DB->sql_compare_text('?')."
AND (f.component <> ? OR f.filearea <> ?)";

$rs = $DB->get_recordset_sql($sql, array($reference, 'user', 'draft'));
$files = array();
Expand Down Expand Up @@ -1769,7 +1772,8 @@ public function get_references_count_by_storedfile($storedfile) {
FROM {files} f
LEFT JOIN {files_reference} r
ON f.referencefileid = r.id
WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
WHERE ".$DB->sql_compare_text('r.reference').' = '.$DB->sql_compare_text('?')."
AND (f.component <> ? OR f.filearea <> ?)";

$count = $DB->count_records_sql($sql, array($reference, 'user', 'draft'));
return $count;
Expand Down Expand Up @@ -1875,5 +1879,37 @@ public function cron() {
mtrace('done.');
}
}

/**
* Get the sql formated fields for a file instance to be created from a
* {files} and {files_refernece} join.
*
* @param string $filesprefix the table prefix for the {files} table
* @param string $filesreferenceprefix the table prefix for the {files_reference} table
* @return string the sql to go after a SELECT
*/
private static function instance_sql_fields($filesprefix, $filesreferenceprefix) {
// Note, these fieldnames MUST NOT overlap between the two tables,
// else problems like MDL-33172 occur.
$filefields = array('contenthash', 'pathnamehash', 'contextid', 'component', 'filearea',
'itemid', 'filepath', 'filename', 'userid', 'filesize', 'mimetype', 'status', 'source',
'author', 'license', 'timecreated', 'timemodified', 'sortorder', 'referencefileid',
'referencelastsync', 'referencelifetime');

$referencefields = array('repositoryid', 'reference');

// id is specifically named to prevent overlaping between the two tables.
$fields = array();
$fields[] = $filesprefix.'.id AS id';
foreach ($filefields as $field) {
$fields[] = "{$filesprefix}.{$field}";
}

foreach ($referencefields as $field) {
$fields[] = "{$filesreferenceprefix}.{$field}";
}

return implode(', ', $fields);
}
}

0 comments on commit 3447100

Please sign in to comment.