From 0494de7a1eeef16a4481d5e3e1fd8967298063b0 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 12 Jul 2015 12:19:28 +0200 Subject: [PATCH] MDL-50771 oracle: Bind long (>4000byte) text as CLOB While we already were detecting @ normalise_value() both BLOBs and CLOBs properly on insert and update statements (by introspectind the column specs), when a long TEXT is being passed as part of an arbitrary SQL, or within the conditions... it was being bound as VARCHAR, leading to problems if used in combination with DBMS_LOB functions. This patch just ensures that any TEXT > 4000 bytes not detected by normalise_value() will be, always, bound as CLOB, unconditionally. Covered with tests both covering its use with DBMS_LOB (the reported bug) and also in general raw statements. --- lib/dml/oci_native_moodle_database.php | 14 ++++++++++- lib/dml/tests/dml_test.php | 34 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/dml/oci_native_moodle_database.php b/lib/dml/oci_native_moodle_database.php index e04ada7a61e..d8f8d67c60c 100644 --- a/lib/dml/oci_native_moodle_database.php +++ b/lib/dml/oci_native_moodle_database.php @@ -683,7 +683,7 @@ class oci_native_moodle_database extends moodle_database { if (is_bool($value)) { // Always, convert boolean to int $value = (int)$value; - } else if ($column->meta_type == 'B') { // CLOB detected, we return 'blob' array instead of raw value to allow + } else if ($column->meta_type == 'B') { // BLOB detected, we return 'blob' array instead of raw value to allow if (!is_null($value)) { // binding/executing code later to know about its nature $value = array('blob' => $value); } @@ -945,6 +945,18 @@ class oci_native_moodle_database extends moodle_database { $descriptors[] = $lob; continue; // Column binding finished, go to next one } + } else { + // If, at this point, the param value > 4000 (bytes), let's assume it's a clob + // passed in an arbitrary sql (not processed by normalise_value() ever, + // and let's handle it as such. This will provide proper binding of CLOBs in + // conditions and other raw SQLs not covered by the above function. + if (strlen($value) > 4000) { + $lob = oci_new_descriptor($this->oci, OCI_DTYPE_LOB); + oci_bind_by_name($stmt, $key, $lob, -1, SQLT_CLOB); + $lob->writeTemporary($this->oracle_dirty_hack($tablename, $columnname, $params[$key]), OCI_TEMP_CLOB); + $descriptors[] = $lob; + continue; // Param binding finished, go to next one. + } } // TODO: Put proper types and length is possible (enormous speedup) // Arrived here, continue with standard processing, using metadata if possible diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php index 0863f30c444..748f11d529d 100644 --- a/lib/dml/tests/dml_test.php +++ b/lib/dml/tests/dml_test.php @@ -858,6 +858,7 @@ class core_dml_testcase extends database_driver_testcase { $tablename2 = $table2->getName(); $table2->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); $table2->add_field('course', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + $table2->add_field('onetext', XMLDB_TYPE_TEXT, 'big', null, null, null); $table2->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); $dbman->create_table($table2); @@ -904,6 +905,30 @@ class core_dml_testcase extends database_driver_testcase { FROM {{$tablename1}}"; $this->assertTrue($DB->execute($sql)); $this->assertEquals(4, $DB->count_records($tablename2)); + + // Insert a TEXT with raw SQL, binding TEXT params. + $course = 9999; + $onetext = file_get_contents(__DIR__ . '/fixtures/clob.txt'); + $sql = "INSERT INTO {{$tablename2}} (course, onetext) + VALUES (:course, :onetext)"; + $DB->execute($sql, array('course' => $course, 'onetext' => $onetext)); + $records = $DB->get_records($tablename2, array('course' => $course)); + $this->assertCount(1, $records); + $record = reset($records); + $this->assertSame($onetext, $record->onetext); + + // Update a TEXT with raw SQL, binding TEXT params. + $newcourse = 10000; + $newonetext = file_get_contents(__DIR__ . '/fixtures/clob.txt') . '- updated'; + $sql = "UPDATE {{$tablename2}} SET course = :newcourse, onetext = :newonetext + WHERE course = :oldcourse"; + $DB->execute($sql, array('oldcourse' => $course, 'newcourse' => $newcourse, 'newonetext' => $newonetext)); + $records = $DB->get_records($tablename2, array('course' => $course)); + $this->assertCount(0, $records); + $records = $DB->get_records($tablename2, array('course' => $newcourse)); + $this->assertCount(1, $records); + $record = reset($records); + $this->assertSame($newonetext, $record->onetext); } public function test_get_recordset() { @@ -3724,6 +3749,15 @@ class core_dml_testcase extends database_driver_testcase { $this->assertCount(1, $records); } + // Now test the function with really big content and params. + $clob = file_get_contents(__DIR__ . '/fixtures/clob.txt'); + $DB->insert_record($tablename, array('name' => 'zzzz', 'description' => $clob)); + $sql = "SELECT * FROM {{$tablename}} + WHERE " . $DB->sql_compare_text('description') . " = " . $DB->sql_compare_text(':clob'); + $records = $DB->get_records_sql($sql, array('clob' => $clob)); + $this->assertCount(1, $records); + $record = reset($records); + $this->assertSame($clob, $record->description); } public function test_unique_index_collation_trouble() {