As case mentioned in #147947, here is a split patch for remap {aggregator_item}.description as BLOB type. Also update some missing default '' value for text fields. Using drupal_write_record() since implementation can be more simple then using db_insert() or db_update().

Patch via CVS HEAD, tested with MySQL (successful), PostgreSQL (fail after update, as like as case of CVS HEAD) and Oracle (successful with same programming logic via my research project). Update script also function with MySQL.

Comments

hswong3i’s picture

StatusFileSize
new4.14 KB

Fix missing default value update script.

hswong3i’s picture

StatusFileSize
new5.27 KB

Also replace all nearby queries as new standard. Tested with MySQL.

hswong3i’s picture

Priority: Normal » Critical
Status: Needs review » Needs work
StatusFileSize
new3.46 KB

Sorry, just back to the main focus and patch for BLOB related only.

BTW, it is know as buggy for MySQL when db_insert() + BLOB + db_last_insert_id() (just after insert). It is because MySQL's db_insert() now handling with DELAYED default on (http://dev.mysql.com/doc/refman/5.0/en/insert-delayed.html):

Because the INSERT DELAYED statement returns immediately, before the rows are inserted, you cannot use LAST_INSERT_ID() to get the AUTO_INCREMENT value that the statement might generate.

The call of db_insert() and db_update() is successful when working with original schema (text). On the other hand, when handling with drupal_write_record(), the raw query builder without DELAYED on, both insert and update will also successful. The programming logic itself is functional, it should be the bug of our new DB abstraction layer. I will report this bug in other issue.

P.S. one more bug... I guess by definition, field with 'not null' => TRUE should come with default value; BTW, in case of MySQL + TEXT/BLOB, default value is not allowed (http://dev.mysql.com/doc/refman/5.0/en/blob.html, http://bugs.mysql.com/bug.php?id=19498). This may also need to handle within another issue, too :S

hswong3i’s picture

Status: Needs work » Needs review
StatusFileSize
new4.31 KB

Reroll patch without db_last_insert_id(), and also merge with http://drupal.org/node/300219 for correct schema definition.

MySQL:

  • update.php: pass.
  • Fetch flash new RSS data (INSERT): pass.
  • Update existing RSS data: pass.

PostgreSQL:

  • update.php: FAILED. Seems PostgreSQL schema don't handle change field correctly.
  • Fetch flash new RSS data (INSERT): Partly pass as CVS HEAD. Items are fetched but page will become blank.
  • Update existing RSS data: as like as case INSERT.
hswong3i’s picture

StatusFileSize
new3.79 KB

Revamp with drupal_write_record() for simpler handling.

When combine with simplify drupal_write_record() with db_insert() and db_update(), it can prevent the incorrect input type for pid and uid when compare with using db_insert() directly.

Tested with MySQL and PostgreSQL. Stand alone simpletetest, pass; simpletest when combine with http://drupal.org/node/299088, also pass.

hswong3i’s picture

Update based on http://drupal.org/node/316095 founding. Revamp BLOB field with nullable.

Tested with MySQL and PostgreSQL. Stand alone simpletest, pass; simpletest when combine with http://drupal.org/node/299088, also pass.

hswong3i’s picture

Fix incorrect upgrade path.

In case of PostgreSQL, we need to combine with http://drupal.org/node/299088 in order to get db_insert/db_update for BLOB correctly within drupal_write_record.

hswong3i’s picture

Title: Remap field as BLOB: {aggregator_item}.description » [DBTNG + BLOB]: remap {aggregator_item}.description
StatusFileSize
new3.23 KB

Since #316095: Raise PHP requirement to 5.2.12 for PostgreSQL only already figure out the solution for PostgreSQL + BLOB + NULL + INSERT/UPDATE bug, this patch is now safe for using both null or nullable BLOB field.

Patch reroll via CVS HEAD. Only change field type from TEXT to BLOB.

P.S. Actually, I would like to add default value for "variables" as it is 'not null' => TRUE by default. But since MySQL will buggy with #300219: [DBTNG]: MySQL should remove TEXT/BLOB default value so I would like to wait and handle this with another issue.

hswong3i’s picture

Title: [DBTNG + BLOB]: remap {aggregator_item}.description » [DBTNG + BLOB] remap {aggregator_item}.description
StatusFileSize
new1.55 KB

Patch update via CVS HEAD, running simpletest with MySQL correctly.

Status: Needs review » Needs work

The last submitted patch failed testing.

lilou’s picture

Status: Needs work » Needs review
hswong3i’s picture

Status: Needs review » Closed (duplicate)