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.

CommentFileSizeAuthor
#13 driver_cleanup-0.5.1.patch99.51 KBhswong3i
#12 driver_cleanup-0.5.patch99.68 KBhswong3i
#11 driver_cleanup-0.4.patch97.25 KBhswong3i
#3 driver_cleanup-0.3.patch107.36 KBhswong3i
#2 driver_cleanup-0.2.patch107.22 KBhswong3i
driver_cleanup-0.1.patch103.77 KBhswong3i

Comments

hswong3i’s picture

hswong3i commented
Title: db drivers code style cleanup » Siren #7: db drivers code style cleanup
Assigned: Unassigned » hswong3i

I mark this into my personal research project issue.

hswong3i’s picture

hswong3i commented
Title: Siren #7: db drivers code style cleanup » Siren #7: DB drivers cleanup + split Schema API and common functions
StatusFileSize
new driver_cleanup-0.2.patch107.22 KB

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.
hswong3i’s picture

hswong3i commented
Title: Siren #7: DB drivers cleanup + split Schema API and common functions » Siren #7: split Schema API and shared functions from DB drivers
StatusFileSize
new driver_cleanup-0.3.patch107.36 KB

Patch reroll based on the change of #178523.

catch’s picture

catch
he/him
Primary language English
commented
Version: 6.x-dev » 7.x-dev
hswong3i’s picture

hswong3i commented

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

catch’s picture

catch
he/him
Primary language English
commented

107.36 KB and these.

hswong3i’s picture

hswong3i commented

@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 :-)

catch’s picture

catch
he/him
Primary language English
commented

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

chx’s picture

chx commented
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.

bjaspan’s picture

bjaspan commented

subscribe

hswong3i’s picture

hswong3i commented
StatusFileSize
new driver_cleanup-0.4.patch97.25 KB

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.

hswong3i’s picture

hswong3i commented
Priority: Normal » Critical
StatusFileSize
new driver_cleanup-0.5.patch99.68 KB

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

hswong3i’s picture

hswong3i commented
StatusFileSize
new driver_cleanup-0.5.1.patch99.51 KB

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.

dropiopio’s picture

dropiopio commented
Priority: Critical » Normal
Status: Needs review » Fixed

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

hswong3i’s picture

hswong3i commented
Status: Fixed » Needs review
dries’s picture

dries commented
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?

hswong3i’s picture

hswong3i commented

@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

Crell’s picture

Crell commented
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.

hswong3i’s picture

hswong3i commented
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.

Crell’s picture

Crell commented
Status: Needs review » Closed (won't fix)

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