[Owncloud] oracle support

[Jörn Friedrich Dreyer]

On 23.07.2012 22:35, Thomas Müller wrote:
> Am Montag, dem 23.07.2012 um 22:28 schrieb Jörn Friedrich Dreyer:
>> On 23.07.2012 21:32, Thomas Müller wrote:
>>> Look slike MDB2 has a solution for this:
>>> http://pear.php.net/manual/en/package.database.mdb2.intro-quote.php
>> MDB2 has, and we use it when creating the database. But PDO has not and
>> we use it for any query after creating the database, breaking any query
>> that does not use backticks ...
>>
> Why are there two database components (MDB2 and PDO) in place?
> Let's use MDB2 trough out the code and we will be fine - whatever database support we need.
Actually, ownCloud has an OC_DB class that transparently chooses the 
backend (MDB2 is default, PDO if available). I hacked the class to 
always use MDB2 with oracle but that does not solve the problem of 
unescaped identifiers in SQL statements. MDB2 provides a quoteIdentifier 
method that is meant to be used when constructing SQL queries like this:

$query = 'SELECT * FROM ' . $mdb2->quoteIdentifier('users', true).
          ' WHERE ' . $mdb2->quoteIdentifier('user', true) . ' = ?';

Aside from being completely unreadable I would still need to check every 
SQL statement in ownCloud. I then prefer to add backticks to the 
statements and use OC_DB::processQuery to switch them to the database 
specific one. The above query would look like this:

$query = 'SELECT * FROM `users` WHERE `user` = ?';

Both queries are used as a prepared statement.

Yes, I will need to touch every query and we will have to add the 
practice to the coding guidelines. But at least developers do not need 
to learn a new way to interact with the database in ownCloud.

That would be the case for the DB back-end Diederik proposed. It hides 
the SQL and provides methods for CRUD operations. While I like the idea, 
writing this kind of backend requires a lot of time and might need a few 
iterations to become stable.

The last option would be to rename any identifiers that collide with 
reserved words from databases (Oracle has USER, UID, SIZE and lots of 
others). Then we can strip the back-ticks from all queries but have to 
keep in mind that some words are not allowed as a table or column name.

Or we can prefix column names with 'oc_' ... I don't know what to think 
of that ...

Personally, I think adding back-ticks is at least consistent and has 
minimal impact on the current db layer. Yes, it is diligence work but it 
won't force developers to learn new stuff. We could even check for 
backticks in OC_DB::processQuery and throw a warning so new developers 
get a chance to learn they should use back-ticks.

I seriously hope I am just missing something. Any other ideas are 
welcome! Anyone?

so long

Jörn

>   
>> thx
>>> Take care,
>>>
>>> Tom
>>>
>>> Am Montag, dem 23.07.2012 um 20:12 schrieb Jörn Friedrich Dreyer:
>>>> The problem: Oracle up-cases identifiers in queries that have not been
>>>> quoted:
>>>>
>>>> SELECT user FROM oc_fscache
>>>>
>>>> becomes
>>>>
>>>> SELECT USER FROM OC_FSCACHE;
>>>>
>>>> This causes problems as some of our queries use quotes and some don't,
>>>> resulting in strange errors where oracle dies on comparing strings with
>>>> integers ... yeah, welcome to debugging hell.
>>>>
>>>> I already tried removing back ticks in OC_DB::processQuery() and setting
>>>> 'quote_identifier' = false for MDB2.  The idea was to use oracles
>>>> automatic uppercase conversion. MDB2 would not quote table names and
>>>> columns and oracle would create them up-cased. PDO statements would be
>>>> stripped of quotes and no further modifications would have been
>>>> necessary. Unfortunately, Oracle disallows the use of some registered
>>>> words like 'USER', 'SIZE', 'UID', and maybe others we use as column names.
>>>>
>>>> The only other viable option I now see is forcing the use of back ticks
>>>> and lowercase for table AND column names. Not only when creating the
>>>> database with MDB2 (which is already the case) but in all queries.
>>>> Lowercase versions of user, uid and size do not collide with Oracle
>>>> reserved words. OC_DB::processQuery() can then change the quotation if
>>>> needed before executing a query with PDO. So, the general way things
>>>> work would not change. However, I would need to fix at least 248 matches
>>>> of ' *PREFIX*' and even more occurances of unescaped column names. In
>>>> the future we might add a code smell for unescaped SQL identifiers in
>>>> our CI server (hint hint ... this is a very good use case for CI by the
>>>> way).
>>>>
>>>> I admit that adding backticks seems superflous and to my personal taste
>>>> even hinders readability, but I don't see another sane solution to
>>>> support oracle.
>>>>
>>>> Maybe someone of you does?
>>>>
>>>> so long
>>>>
>>>> Jörn
>>>>
>>>> -- 
>>>> Jörn Friedrich Dreyer (jfd at owncloud.com)
>>>> Software Developer
>>>> ownCloud GmbH
>>>>
>>>> Your Data, Your Cloud, Your Way!
>>>>
>>>> ownCloud GmbH, GF: Markus Rex, Holger Dyroff
>>>> Schloßäckerstrasse 26a, 90443 Nürnberg, HRB 28050 (AG Nürnberg)
>>>>
>>>> _______________________________________________
>>>> Owncloud mailing list
>>>> Owncloud at kde.org
>>>> https://mail.kde.org/mailman/listinfo/owncloud
>>
>> -- 
>> Jörn Friedrich Dreyer (jfd at owncloud.com)
>> Software Developer
>> ownCloud GmbH
>>
>> Your Data, Your Cloud, Your Way!
>>
>> ownCloud GmbH, GF: Markus Rex, Holger Dyroff
>> Schloßäckerstrasse 26a, 90443 Nürnberg, HRB 28050 (AG Nürnberg)


-- 
Jörn Friedrich Dreyer (jfd at owncloud.com)
Software Developer
ownCloud GmbH

Your Data, Your Cloud, Your Way!

ownCloud GmbH, GF: Markus Rex, Holger Dyroff
Schloßäckerstrasse 26a, 90443 Nürnberg, HRB 28050 (AG Nürnberg)