Joomla! Issue Tracker | Joomla! CMS #12588 - OCI8 Versions of Oracle Database Driver, Iterator, and Query Classes

Closed
29 Jun 2019
Medium
Build: staging
# 12588
Diff
orware:issue-12560

Failure

Success continuous-integration/travis-ci/pr The Travis CI build passed Details
Failure continuous-integration/appveyor/pr AppVeyor build failed Details
Error continuous-integration/jenkins/pr-merge This commit cannot be built Details

User tests: Successful: Unsuccessful:

orware
27 Oct 2016

Pull Request for Issue #12560.

Summary of Changes

Renaming of existing PDO Oracle Driver, Iterator, and Query classes to pdooracle.php.
Creation of new OCI8 versions of Oracle Driver, Iterator, and Query classes which are named oracle.php

This primarily came up because I've been wanting to do some Joomla work that will need to utilize an Oracle database for some data retrieval, but PHP 7's support for the pdo_oci extension is not very good and it's unclear whether or not that will improve, so in order to better support PHP 7, and things that work well with it, it's best that we have an OCI8 version of the Oracle Driver, Iterator and Query classes available.

I'm not sure if the Framework's Database package is being actively maintained, but I could submit these changes over there as well too if needed.

Testing Instructions

This is going to be difficult for most, since not everybody has an Oracle database available to test with and this would really only apply to the few developers utilizing Joomla in this fashion to connect to external Oracle databases from within their custom extensions inside of a Joomla site.

I almost want to say that it would simply be easiest if the submissions could be reviewed for general correctness, but not necessarily for functional correctness due to the difficulty doing so might require.

However, generally on Windows with XAMPP and the 32-bit version of the Oracle instant client downloaded and added to the Windows Path, it is pretty easy to be able to enable your local PHP install for Oracle support. The next step would be to grab a copy of Oracle's Express Edition and install that onto your local machine or a VM, and with the sample database, connect to it from within Joomla using something like this (I added this code right before the closing body tag in the protostar template of a default Joomla install for testing from my end):

$options = [
    'database' => 'TEST',
    'user' => 'your_username',
    'password' => 'your_password',
    'host' => 'localhost',
    'driver' => 'oracle',
];
?>
<?php $oracle = JDatabase::getInstance($options); ?>
<?php
    //$result = $oracle->connected();
    //$rows = $oracle->getTableColumns('SOME_TABLE', false);
    //$rows = $oracle->getTableCreate('SOME_SCHEMA.SOME_TABLE');

    //$rows = $oracle->getTableKeys('SOME_TABLE');

    //$rows = $oracle->getTableList(null, true);
    $rows = $oracle->getTableList('SOME_SCHEMA', true);

    //$rows = $oracle->getVersion();
    //$oracle->setDateFormat();
    //$oracle->setQuery('SELECT * FROM SOME_TABLE WHERE SOME_ID = 123');

    /*
    $pidm = 123;
    $query = $oracle->getQuery(true);

    $query->select('*');
    $query->from('SOME_TABLE');
    $query->where('SOME_ID = :some_id');

    $query->bind(':some_id', $some_id);

    $oracle->setQuery($query);
    */

    //$oracle->setQuery('SELECT * FROM SOME_TABLE WHERE SOME_ID = :some_id');
    //$oracle->getQuery()->bind(':some_id', $some_id);

    //$oracle->setQuery('select * from SOME_TABLE', 0, 100);

    /*
    $oracle->toUpper();
    $it = $oracle->getIterator(null, 'ArrayObject');

    $rows = array();
    foreach($it as $row)
    {
        $rows[] = $row;
    }
    */

    //$rows = $oracle->loadObjectList();

    //$affected = $oracle->getAffectedRows();
    //$numRows = $oracle->getNumRows();

    //$result2 = $oracle->connected();
    echo "<pre>";
    echo print_r($rows, true);
    echo "</pre>";
?>

More info on what I've tested so far can be found over on the Issue #12560 page and I'm happy to answer questions that might come up too.

I'm pretty sure this initial PR will fail due to code consistency checks, so I'll work on correcting those tomorrow morning.

Documentation Changes Required

f031ec3 27 Oct 2016

Renamed oracle.php to pdooracle.php

3f6c04d 27 Oct 2016

Work in progress for oci8 version of oracle.php

073be8c 27 Oct 2016

Renaming oracle.php Query file to pdooracle.php

3a36e03 27 Oct 2016

Renaming oracle.php Iterator to pdooracle.php

ead47c3 27 Oct 2016

Renaming the class

93372e3 27 Oct 2016

Basic implementation of oci8 iterator class.

25d5311 27 Oct 2016

Adding back in an oracle.php Query class

8a98020 27 Oct 2016

Bugfix - changed default length parameter to be -1 instead of 0.

b229cf2 27 Oct 2016

Added the oracle.php Iterator class and brought in support for lower casing the field names and returning LOB field values.

46a814e 27 Oct 2016

Customized the getIterator() method so that it passes in the field case / Return LOBs settings to the Iterator from the Database object.

2ba615c 27 Oct 2016

Improved getTableCreate() and added support for schema to be provided and automatic upper casing of provided input to help ensure things work properly (e.g. MY_SCHEMA.TABLE_NAME would work as well as TABLE_NAME which would search for that table within the current user's schema).

430526b 27 Oct 2016

Additional code cleanup and small improvements.

870dc8b 27 Oct 2016

Improved getNumRows() and getAffectedRows() and minor comment cleanup

db57bed 27 Oct 2016

Worked on getTableColumns()

aff7abf 27 Oct 2016

Ported over some changes made to oracle.php to pdooracle.php

b3c9c5b 27 Oct 2016

Improved the PDO Oracle Driver's getTableCreate() method.

orware - open - 27 Oct 2016

orware - change - 27 Oct 2016

Status

New

⇒

Pending

joomla-cms-bot - change - 27 Oct 2016

Labels

Added: ?

joomla-cms-bot - change - 27 Oct 2016

Category

⇒

Libraries

51a30d1 27 Oct 2016

Fixing as many Codesniffer errors as I can so I can recheck again.

767e668 27 Oct 2016

Fixing additional Codesniffer errors (convert tabs to spaces).

mbabker - comment - 27 Oct 2016

So this really isn't backward compatible, even if next to nobody uses this code. Without a configuration change, someone using the existing Oracle driver would go from the PDO connection to the new class.

For the "new" PDO class, you'll also have to add a new case to the existing switch in the base JDatabaseDriverPdo::connect() method for the pdooracle case (the driver value in the options array is used by JDatabaseDriver::getInstance() to derive the correct subclass and does not necessarily align with the cases in that method).

orware - comment - 27 Oct 2016

Hi Michael,

Thanks for the feedback :-).

That's definitely true, due to slight differences between the two it wouldn't truly be 100% backwards compatible. I mainly took what seemed to be the path that made the most sense to me, but perhaps we could also leave the "new" PDO named files back with their old names and use oci8 as the name for the ones that use the OCI8 functions to keep things backwards compatible if that helps meet that requirement.

I'm not sure how much backwards compatibility would be needed in this particular case though, so I'm open to any suggestions that would help make it easier to approve/include.

For the second piece of feedback regarding the switch statement, I don't beleive that's necessary, at least not from the tests I was doing last night on PHP 5.6 at home (where I have both oci8 and pdo_oci extensions up and running) since I was switching between the two and making sure both were still working properly, and during those debugging sessions Joomla was able to find the PDO Oracle driver/iterator/query files normally when I provided "pdooracle" as the driver name when creating the instance and correctly found the OCI8 one when I provided "oracle" as the driver name.

When providing "pdooracle" as the driver name this satisfies what JDatabaseDriver::getInstance() needs to find the correct file (and also satisfies what getIterator() and getQuery() needs too since the $name value has been updated to "pdooracle" within that file). Then within JDatabaseDriverPdooracle::__construct() it overrides the driver name and provides "oci" specifically (which is what's needed during the JDatabaseDriverPdo::connect() call when creating the PDO DSN string). So I believe everything's connected properly.

I think that was the main thing you may have been concerned about with this item? That JDatabaseDriver::getInstance() wouldn't fail if I provided "pdooracle" as the driver name? If there's anything else I missed just let me know.

Depending on whether or not I need to rename the PDO versions back to their original names or not to maintain backwards compatibility and come up with new names for OCI8 versions, I'll mainly just continue some testing here on my end to make sure different queries we have on my end that I can throw at the driver to test are working as expected.

mbabker - comment - 27 Oct 2016

Didn't realize the constructor was overriding the value completely instead of checking it like others. If that's the case it'll be fine then.

9d37769 27 Oct 2016

Tested the dropTable() method and made correction. Also added in @ suppression during the oci_execute() call within execute().

8cbe108 27 Oct 2016

Added new getQueryType() method available in oci8 for showing the query type (SELECT/INSERT/UPDATE/DELETE/etc. of the current query passed to setQuery().

ed05a1e 27 Oct 2016

Added error suppression in freeResult() here since it will throw a warning if you don't remember to unset() the variable after using it and you continue to run other queries afterwards. The warning would get thrown in the __destruct() process at the every end of the page execution.

93d0374 27 Oct 2016

Fixing a Codesniffer error for a comment.

5bbc93c 27 Oct 2016

Realized my use of escape() might have been incorrect in dropTable() earlier and switched over to using quoteName() instead and added that as well to the renameTable() method. Also needed to ensure the provided table names were in uppercase since otherwise Oracle wouldn't be able to find the table properly.

6405cf0 27 Oct 2016

Switching the transaction methods to the implementation from the mysqli driver, since it is closer to what's needed (the methods that were in there were more specific to the PDO drivers). Just need to do some testing now.

84745ba 27 Oct 2016

Added extra code for Oracle Transaction process (still testing though). Added feature to prevent dropTable() from throwing an exception if the table does not exist. Added a copyTable() method that can copy a table with or without its data.

b3497e8 27 Oct 2016

Forgot to add the strtoupper calls for the input variables.

a4d2a42 27 Oct 2016

Made some additional minor changes to the transaction methods. Tested both out and they work correctly from what I can tell :-).

2edce2f 27 Oct 2016

Added overrides for loadAssocList() and loadObjectList() that way field name casing could be handled appropriately/automatically.

92c6fc2 27 Oct 2016

Fixing Codesniffer errors.

8a92256 27 Oct 2016

Fixing Codesniffer errors.

orware - comment - 27 Oct 2016

Quite a bit of additional testing/checking was completed throughout today.

Went ahead and installed the Express Edition of Oracle on my work PC here and put together some notes and info on the tests I was running on my end here:
https://github.com/orware/jdatabaseoracle-tests

Testing Updates:

transactionCommit() - Working
transactionRollback() - Working
transactionStart() - Working
dropTable() - Working
lockTable() - Not Tested
renameTable() - Working
unlockTables() - Not Tested
getCreateDatabaseQuery() - Not Tested

Also added in some minor additional functionality:

getQueryType() (OCI8 had a useful function for this built-in)
copyTable() (SQL Developer gave me a hint I could use to make this work to copy an existing table to a new table and optionally include its data).

Also added some error suppression for certain cases using PHP's @ operator or by preventing an exception from being thrown in certain cases (I did this in copyTable() and dropTable()).

I put together a short intro video just to demonstrate it in action for any folks interested in testing it further (I apologize if the audio is a little low...I don't think I was talking very loud as I was putting this together and I was talking somewhat quickly):
https://www.youtube.com/watch?v=nUdElYErCf8&feature=youtu.be

I'll continue running some additional tests later tonight using the PDO version of the driver, but the OCI8 version feels pretty solid now.

orware - comment - 29 Oct 2016

Completed my testing of these remaining methods:

lockTable() - Working (from what I can tell)
unlockTables() - No issues found
getCreateDatabaseQuery() - Quite a bit of extra work here needed to make this one work due to the differences between what MySQL calls a database and how Oracle works (in Oracle, the whole system is typically called a Database Instance, and within this you have multiple schemas, which also correspond to users for the most part and there's quite a few extra GRANTS that need to be made when a new user gets created in order for tables to be allowed to be created, etc. in the new schema).

And also fixed a number of smaller bugs that cropped up during testing today.

The repository over here has been updated with some extra tests I added in as I was doing my extra testing today:
https://github.com/orware/jdatabaseoracle-tests

And I also did back-to-back testing between the OCI8 version of the driver and the PDO Oracle driver throughout today and making them more or less functionally equivalent (I compared the outputs of the tests between the two drivers and there was only a minor difference for Tests 18 and 19, where the OCI8 driver returns 1 for getAffectedRows() and getNumRows() whereas the PDO Oracle driver returns 0 in both cases).

When I originally built a JDatabaseOracle class back in the Joomla 1.5 days, one thing I added in was the ability for the driver to default to lowercase field names when retrieving result sets, since the OCI8 extension only allowed for them to be returned in uppercase format, which I felt was harder to work with. On the PDO side it's technically a bit easier since it's built-in as an option you can pass into your PDO connection, but the lowercase option wasn't turned on by default for the PDO Oracle class. In wanting to make it operate the same as the OCI8 one I've been working on here, I've set the PDO Oracle driver to now default to lowercase as well. I had wanted to integrate that option into the base PDO class, but it seemed to throw off a lot of the unit tests that were using the PDO MySQL driver so I moved those particular changes over to only be in the PDO Oracle driver class so as to avoid any potential changes to the PDO MySQL driver at this time.

Aside from trying to see if there's any sort of improvements I might be able to possibly make to the getAffectedRows() and getNumRows() methods, there's not much else further I think that's needed and the OCI8 driver should be in a very usable state.

orware - change - 1 Nov 2016

The description was changed

joomla-cms-bot - change - 3 Nov 2016