Some database-specific SQL functions abstraction can be handled in simpler style, e.g. predefined constant, based on existing founding in http://drupal.org/node/167335, http://drupal.org/node/172541 and http://edin.no-ip.com/html/?q=node/310. It is found as meaningful for database abstraction, which won't harm any performance.
This is a simplified and backport patch for D6 based on existing founding (http://drupal.org/node/172541), include following changes:
SUBSTRING, LENGTH and CONCAT within comment.module, statistics.admin.inc and locale.module respectivelyrand and concat functions within system.install| Comment | File | Size | Author |
|---|---|---|---|
| #8 | drupal-6.x-dev-abs_sql_func-0.4.patch | 8.37 KB | hswong3i |
| #6 | drupal-6.x-dev-abs_sql_func-0.3.patch | 8.37 KB | hswong3i |
| #1 | drupal-6.x-dev-abs_sql_func-0.2.patch | 7.58 KB | hswong3i |
| drupal-6.x-dev-abs_sql_func-0.1.patch | 7.79 KB | hswong3i |
Comments
Comment #1
hswong3i commentedminor update: abstraction of database escape character should belong to another issue (http://drupal.org/node/371), so remove in here.
Comment #2
chx commentedThis is a feature request and as such it goes into D7. Before you scream bloody murder in another twelve thousand character diatribe carefully spamming my issue AND planet at the same time -- the db_lower patch is a performance fix.
Comment #3
hswong3i commentedSince this patch will able to simplify the use of some database-specific function (e.g. abstract by predefined constant and simple API function), it can decrease the difficult of code maintenance (we don't need to keep on maintain those fancy user-defined functions in pgsql) and also development. Developers will able to fully utilize those SQL-level feature without considering the different syntax among different database, so it improve compatibility. It is not a "hard" bug but "soft". Once again, as we will not died without this patch, so it just goes minor.
On the other hand, those SQL functions are not widely used: it just affect 6 lines among all core modules. It keep existing code function without losing performance, nor introduce complicated handling to developers. The abstraction is somehow simple and functioning for all cases without any database-specific nor case-by-case-specific pre-requirement. We are able to tidy this up without affecting contribute developers as nearly transparency. We are able to fix this on time, even after D6 beta1, so it should belongs to D6 as coding style cleanup.
@chx: since there is no direct relationship of this issue with db_lower() patch, I will not comment about that in here. I just try to propose some counter idea for brainstorming. I didn't change any status of that issue as that shouldn't be my duty. I have no duty about its pass or not. I am just a small potato without this power :)
Comment #4
hswong3i commentedI have review this patch once again, about its usability and impact. It is transparency, handy, don't break any existing code, no performance impact, no bias in any database, supported by a complete research result, and able to simplify some handling for PgSQL. The changes are optional to contribute developer so won't break their works on hand. May be it seems a bit crazy for sliding into D6, but shouldn't we give a little bit love to it?
Anyway, I will come back to this issue later, if it is not suitable for D6: it is just a backport patch from my previous D7 research result. We need not to be hurry about this currently :)
Comment #5
hswong3i commentedComment #6
hswong3i commentedThis patch is now tested for both MySQL 5.0.32 and PostgreSQL 8.1.9 on Debian etch 4.0r1 with PHP 5.2.0-8+etch7, in case of comment setting with "older first" (test for DB_SUBSTR and DB_STRLEN) and also statistics_top_visitors() (test for db_concat()), all passed with no error. Patch is now reroll via latest CVS HEAD.
Hope we can reconsider about this patch's possibility, based on its simplify effect for pgsql implementation, even it really sounds tooooo late within D6 development cycle. This patch is always ready for a trivial commit, in whatever D6/D7 :)
Comment #7
moshe weitzman commentedhswong3i - you are completely out of control. please fix some bugs or keep working on D7 stuff. your justification in this issue is not at all compelling.
Comment #8
hswong3i commented@chx and moshe: sorry for rerolling this issue for D6. As we are still having chance for fine turn D6 DB API, hope we would give some reconsideration for this simple issue, too. This patch won't introduce complicated impact to existing implementation, I don't think we should postpone it, and keep it as an open issue for D7. We can fix this within D6.
Comment #9
hswong3i commentedDon't need to waste time when D6 beta3 is close. For D7, it is duplicated with http://drupal.org/node/167335