Download & Extend
Issues

Split Schema API and shared functions

Posted by hswong3i on December 9, 2007 at 5:59am
Project:Drupal core
Version:7.x-dev
Component:database system
Category:task
Priority:critical
Assigned:hswong3i
Status:closed (won't fix)

Issue Summary

database drivers are split into 3 section:

  1. common.mysql.inc and common.pgsql.inc: host shared function mysql, mysqli and pdo_mysql database engines. Most are related to queries syntax difference.
  2. schema.mysql.inc and schema.pgsql.inc: host Schema API. Split for simplify maintenance and upgrade.
  3. database.mysql.inc, database.mysqli.inc and database.pgsql.inc: host PHP driver specific functions.

Also come with some minor syntax sync between mysql and postgresql, and coding style cleanup.

P.S. The rearrangement is now applying to my personal research project, which is also suitable for D7 PDO development. BTW, no D7 specific changes are added to this patch.

AttachmentSizeStatusTest resultOperations
driver_cleanup-0.1.patch103.77 KBIgnored: Check issue status.NoneNone
Login or register to post comments

Comments

#1

Posted by hswong3i on December 10, 2007 at 5:01am
Title:db drivers code style cleanup» Siren #7: db drivers code style cleanup
Assigned to:Anonymous» hswong3i

I mark this into my personal research project issue.

#2

Posted by hswong3i on December 19, 2007 at 8:14am
Title:Siren #7: db drivers code style cleanup» Siren #7: DB drivers cleanup + split Schema API and common functions

Patch reroll via latest CVS HEAD. Test without ill effect. Some additional changes:

  1. Rename file INSTALL.pgsql.txt as INSTALL.postgreql.txt. As we may have database.pdo_pgsql.inc in D7, it seems not suitable to call document of PostgreSQL (product name) as pgsql (PHP driver name).
  2. Rename constant DRUPAL_MINIMUM_PGSQL as DRUPAL_MINIMUM_POSTGRESQL. Similar idea as above.
AttachmentSizeStatusTest resultOperations
driver_cleanup-0.2.patch107.22 KBIgnored: Check issue status.NoneNone

#3

Posted by hswong3i on December 19, 2007 at 6:28pm
Title:Siren #7: DB drivers cleanup + split Schema API and common functions» Siren #7: split Schema API and shared functions from DB drivers

Patch reroll based on the change of #178523.

AttachmentSizeStatusTest resultOperations
driver_cleanup-0.3.patch107.36 KBIgnored: Check issue status.NoneNone

#4

Posted by catch on December 20, 2007 at 1:06am
Version:6.x-dev» 7.x-dev

#5

Posted by hswong3i on December 20, 2007 at 2:55am

@catch: May I have some detail idea about why we need to postpone a code clean up and splitting ?_?

#6

Posted by catch on December 20, 2007 at 10:22am

107.36 KB and these.

#7

Posted by hswong3i on December 20, 2007 at 10:28am

@catch: agree about the huge size. So I try to split it into 2 lightweight issues: http://drupal.org/node/202563 and http://drupal.org/node/202560, each include some simple file rename which should be more acceptable. I will update this issue once again when they are able to pass though, and the size of this issue will much reduce :-)

#8

Posted by catch on December 20, 2007 at 10:41am

Well I'll let someone else bump those to 7.x, not into fighting hydras.

#9

Posted by chx on December 23, 2007 at 9:21am
Title:Siren #7: split Schema API and shared functions from DB drivers» split Schema API and shared functions from DB drivers

As Siren is not a project on Drupal.org but an unofficial fork, I am removing it from issue titles to avoid confusion.

#10

Posted by bjaspan on January 5, 2008 at 12:44am

subscribe

#11

Posted by hswong3i on February 5, 2008 at 6:05am

Since D7 is now open for public development, and this is the common base for most of the other DB API revamp handling, hope it is now the good timing for review this patch.

Patch reroll via latest CVS HEAD (7.x). Only coming with basic cut & paste handling, plus very minor essential change in db_version() of MySQL. I don't give any other syntax change nor clean up within this patch, so it should able to work as a reference point for further more development.

AttachmentSizeStatusTest resultOperations
driver_cleanup-0.4.patch97.25 KBIgnored: Check issue status.NoneNone

#12

Posted by hswong3i on February 22, 2008 at 7:26am
Priority:normal» critical

Again a cut & paste update, plus very minor syntax clean up.
Also integrate chx's patch in http://drupal.org/node/212918#comment-739728

AttachmentSizeStatusTest resultOperations
driver_cleanup-0.5.patch99.68 KBIgnored: Check issue status.NoneNone

#13

Posted by hswong3i on February 22, 2008 at 7:27am

Patch reroll via CVS HEAD, based on http://drupal.org/cvs?commit=102333
Please refer to http://groups.drupal.org/node/8907#drupal-7.x for detail of design.

AttachmentSizeStatusTest resultOperations
driver_cleanup-0.5.1.patch99.51 KBIgnored: Check issue status.NoneNone

#14

Posted by dropiopio on February 22, 2008 at 7:14pm
Priority:critical» normal
Status:needs review» fixed

Thanks for the last patch.
It address the problem at the correct direction

#15

Posted by hswong3i on February 22, 2008 at 7:32pm
Status:fixed» needs review

#16

Posted by Dries on February 23, 2008 at 7:48am
Priority:normal» critical

We're working on PDO-ification of Drupal 7 -- this patch seems to be deprecated already? I'd prefer for the PDO work to land first.

Crell: any input on timeline?

#17

Posted by hswong3i on February 23, 2008 at 5:31pm

@Dires: I am totally agree about PDO + D7, and the use of PDO can simplify a lot of complicated programming logic, e.g. ibm_db2 vs pdo_ibm BLOB handling, pdo_sqlite for SQLIte v3 support, pdo_mysql for emulated prepare statement handling, etc. I have no point to deprecate the use of PDO.

BTW, the main focus of this patch is all about a split-brain handling, and this model have already been proved as function and logical with Moodle 1.7+ XMLDB and my personal research. After this split, we can handle our next generation DB API chances within common.*.inc or database.*.inc, or even on top of them as much higher level; but schema.*.inc should be isolated for a more elegant handling. This image should be a good refer for the overall idea: http://edin.no-ip.com/html/files/DrupalDBStack_D7_draft.png

#18

Posted by Crell on February 24, 2008 at 12:29am
Status:needs review» closed (duplicate)

The code going on at http://drupal.org/node/225450 already includes schema manipulation split off to separate include files as part of the larger revamp. So yes, this is redundant.

@Dries: chx is working on making the installer play nice with the new setup even as I write this. Once that's done, we can roll a testable patch. The code itself is already in a publicly-accessible svn repo. See the issue above for the link.

#19

Posted by hswong3i on February 29, 2008 at 8:23am
Title:split Schema API and shared functions from DB drivers» Split Schema API and shared functions
Status:closed (duplicate)» needs review

After an indeed review, I think this issues is not redundant.

Since http://drupal.org/node/225450 is a huge issue that even need to host with remote SVN service during development (well, just similar case as http://drupal.org/node/172541), we will need to split it as number of small issues for simple review and quality check. I think it is not a good habit to commit a huge patch without detail and indeed review; on the other hand, that is not our traditional approach, too.

So why don't we keep trace similar idea under single issue, or even commit current strict forward implementation before further more development? Starting from elegant should be much better :-)

P.S. Patch from #13 still apply to CVS HEAD.

#20

Posted by Crell on August 21, 2008 at 10:05pm
Status:needs review» closed (won't fix)

The file structure for the database drivers has now changed completely.

nobody click here