I tried to do a new install, but this time i didn't create the database by hand. And the module crashed because it was looking for prefixed tables ...but they were created unprefixed :(
That's fixed now with this patch.

I took the oportunity to replace all the ugly INT(10) by INT: for portability issue we must avoid the use precision with INT, SMALLINT, TINYINT...

Still for portability, Drupal golden rule is to avoid auto-numbered fields and use it's own sequence function db_next_id($sequence_name). The trick is used for {quiz_result}.rid :) But not for {quiz_question_answer}.aid witch is declared as AUTO_INCREMENT :(
Not fixed : waiting for the code that use it (i think within quiz.module) to be corrected before.

Also, MySQL tables shouldn't be declared MyISAM! Some people are not using that engine with Drupal; they prefer InnoDB for example..
Fixed.

Also avoid default INDEX names and provide explicit ones, otherwise it can hurt when restoring a table/database dump..
Fixed.

Last thing: never trust the database default behaviour; it's not portable. So one should always provide own DEFAULT values, specially when columns are NOT NULL.
Not fixed: checks are performed by the code in the module.

Begun: Posgres support :)
Added: Uninstall hook :)

I had a question, but can't remember. Maybe later. :/

CommentFileSizeAuthor
#5 quiz_installer.patch11.48 KBseanbfuller
#4 prefix_serials.patch1.81 KBgilcot
#3 install.patch_0.txt10.48 KBgilcot
install.patch.txt9.04 KBgilcot

Comments

seanbfuller’s picture

seanbfuller commented
Status: Active » Needs review

Good catches. I'll try to take a crack at testing it soon, but I'm pretty snowed under right now. If you are looking to take another pass at it, just let me know when you're ready and I'll test and commit.

gilcot’s picture

gilcot commented

catched! :))

like here (in quiz.module):

 //good
  // Insert quiz_results record
  $rid = db_next_id('quiz_results_rid');
  $now = time();
  $result = db_query("INSERT INTO {quiz_result} (rid, quiz_nid, uid, time_start) VALUES (%d, %d, %d, %d)", 
    $rid, $nid, $uid, $now);

there should be (in multichoice.module @ hook_update() & hook_insert()):

 //good
    } else if (trim($value['answer']) != "") {
      //If there is an answer, insert a new row
      $aid = db_next_id('quiz_question_answer_aid');
      db_query("INSERT INTO {quiz_question_answer} (aid, question_nid, answer, feedback, points) VALUES(%d, %d, '%s', '%s', %d)", 
        $aid, $node->nid, $value['answer'], $value['feedback'], $value['correct']);
    }

or shorter (in multichoice.module @ hook_update() & hook_insert()):

 //good
      db_query("INSERT INTO {quiz_question_answer} (aid, question_nid, answer, feedback, points) VALUES(%d, %d, '%s', '%s', %d)", 
        db_next_id('quiz_question_answer_aid'), $node->nid, $value['answer'], $value['feedback'], $value['correct']);

but not (still in multichoice.module @ hook_update() & hook_insert()):

 //bad
      db_query("INSERT INTO {quiz_question_answer} (question_nid, answer, feedback, points) VALUES(%d, '%s', '%s', %d)", 
        $node->nid, $value['answer'], $value['feedback'], $value['correct']);

patches (patch for module and final install) coming as soon as possible.

gilcot’s picture

gilcot commented
StatusFileSize
new install.patch_0.txt10.48 KB

job done: here is the patch.

gilcot’s picture

gilcot commented
StatusFileSize
new prefix_serials.patch1.81 KB

it's related to prefixing http://drupal.org/node/90325 :)
what's the problem then? in a multi-site installation, the keys conflict... they should be per table; prefixed then.
but i post it here because it should have been done here (this patch should be after the previous one).

seanbfuller’s picture

seanbfuller commented
Status: Needs review » Fixed
StatusFileSize
new quiz_installer.patch11.48 KB

Committed to head.

Attached is a combined patch with one small change: I changed {quiz_results_rid} to {quiz_results}_rid. Minor change, but this seemed to be how node.module was doing it. Seems to make sense, since it is the table that you want to match up with the prefix.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

  • Commit 9240bca on 4.7.x-1.x, 5.x-1.x, 5.x-2.x, 6.x-2.x, 6.x-3.x, 6.x-4.x, 6.x-5.x, 6.x-6.x, 7.x-4.x, master, 7.x-5.x by seanbfuller: 
    #93253 by gilcot: Fixes several table prefix and abstraction issues.

  • Commit 9240bca on 4.7.x-1.x, 5.x-1.x, 5.x-2.x, 6.x-2.x, 6.x-3.x, 6.x-4.x, 6.x-5.x, 6.x-6.x, 7.x-4.x, master, 7.x-5.x, quiz-pages by seanbfuller: 
    #93253 by gilcot: Fixes several table prefix and abstraction issues.

  • Commit 9240bca on 4.7.x-1.x, 5.x-1.x, 5.x-2.x, 6.x-2.x, 6.x-3.x, 6.x-4.x, 6.x-5.x, 6.x-6.x, 7.x-4.x, master, 7.x-5.x, quiz-pages, 2269219 by seanbfuller: 
    #93253 by gilcot: Fixes several table prefix and abstraction issues.

  • Commit 9240bca on 4.7.x-1.x, 5.x-1.x, 5.x-2.x, 6.x-2.x, 6.x-3.x, 6.x-4.x, 6.x-5.x, 6.x-6.x, 7.x-4.x, master, 7.x-5.x, 2269219 by seanbfuller: 
    #93253 by gilcot: Fixes several table prefix and abstraction issues.