[DBTNG + BLOB] remap {aggregator_item}.description

hswong3i - August 24, 2008 - 17:32
Project:Drupal
Version:7.x-dev
Component:aggregator.module
Category:feature request
Priority:critical
Assigned:hswong3i
Status:duplicate
Description

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.

AttachmentSize
aggregator_item.patch3.64 KB
Testbed results
aggregator_item.patchfailedFailed: Failed to apply patch. Detailed results

#1

hswong3i - August 24, 2008 - 18:50

Fix missing default value update script.

AttachmentSize
aggregator_item-description.patch 4.14 KB
Testbed results
aggregator_item-description.patchfailedFailed: Failed to apply patch. Detailed results

#2

hswong3i - August 25, 2008 - 17:37

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

AttachmentSize
aggregator_item-description.patch 5.27 KB
Testbed results
aggregator_item-description.patchfailedFailed: Failed to apply patch. Detailed results

#3

hswong3i - August 26, 2008 - 16:39
Priority:normal» critical
Status:needs review» needs work

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

AttachmentSize
aggregator_item-description.patch 3.46 KB
Testbed results
aggregator_item-description.patchfailedFailed: Failed to apply patch. Detailed results

#4

hswong3i - August 27, 2008 - 19:10
Status:needs work» needs review

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.

AttachmentSize
aggregator_item-description.patch 4.31 KB
Testbed results
aggregator_item-description.patchfailedFailed: Failed to apply patch. Detailed results

#5

hswong3i - September 24, 2008 - 17:46

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.

AttachmentSize
aggregator_item-description.patch 3.79 KB
Testbed results
aggregator_item-description.patchfailedFailed: Failed to apply patch. Detailed results

#6

hswong3i - October 8, 2008 - 17:41

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.

AttachmentSize
aggregator_item-description-1223484147.patch 3.28 KB
Testbed results
aggregator_item-description-1223484147.patchfailedFailed: Failed to apply patch. Detailed results

#7

hswong3i - October 13, 2008 - 05:25

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.

AttachmentSize
aggregator_item-description-1223875261.patch 3.31 KB
Testbed results
aggregator_item-description-1223875261.patchfailedFailed: Failed to apply patch. Detailed results

#8

hswong3i - October 14, 2008 - 16:08
Title:Remap field as BLOB: {aggregator_item}.description» [DBTNG + BLOB]: remap {aggregator_item}.description

Since #316095: [DBTNG + pgsql] db_insert/db_update buggy with empty string input and BLOB field 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.

AttachmentSize
dbtng-aggregator_item-description-1224000437.patch 3.23 KB
Testbed results
dbtng-aggregator_item-description-1224000437.patchfailedFailed: Failed to apply patch. Detailed results

#9

hswong3i - October 31, 2008 - 18:38
Title:[DBTNG + BLOB]: remap {aggregator_item}.description» [DBTNG + BLOB] remap {aggregator_item}.description

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

AttachmentSize
dbtng-aggregator_item-description-1225478216.patch 1.55 KB
Testbed results
dbtng-aggregator_item-description-1225478216.patchpassedPassed: 7386 passes, 0 fails, 0 exceptions Detailed results

#10

System Message - November 16, 2008 - 21:50
Status:needs review» needs work

The last submitted patch failed testing.

#11

lilou - November 17, 2008 - 14:24

#12

hswong3i - November 24, 2008 - 18:59
Status:needs review» duplicate

duplicate with #147947: [DBTNG + XDB] Replace some TEXT:BIG with BLOB

 
 

Drupal is a registered trademark of Dries Buytaert.