Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oci8 improvements #5

Closed
wants to merge 2 commits into from
Closed

Oci8 improvements #5

wants to merge 2 commits into from

Conversation

milokmet
Copy link
Contributor

Hi!

I made some OCI8 improvements, that should be added into doctrine/master. I described them in commit logs.

Here they are:

Added new parameters into OCI8Connection constructor - $charset and $session_mode
Charset is 4th parameter of oci_connect and cannot be specified as part of TNS string like in PDOOci.
I've added session_mode param, because OCI_DEFAULT mode is insufficient when new user/database will be created and it is necessary to connect as OCI_SYSDBA. I saw some CREATE USER statements in schema manager, so this param is just in case.

Fixed error handling in OCI8Connection. When oci_connect failed, $_dbh is boolean, so cannot be used in oci_error();

OCI8Statement is using for all statements OCI_DEFAULT which is the same like OCI_NO_AUTO_COMMIT - therefor any changes that are made without transaction are rolled back.

Now the OCI8 Driver act like any other driver - commit after execute by default, and starting transaction when invoked $conn->beginTransaction()

Thank you for mergin the changes into core.

…session_mode

Charset is 4th parameter of oci_connect and cannot be specified as part of TNS string like in PDOOci.
I've added session_mode param, because OCI_DEFAULT mode is insufficient when new user/database will be created and it is necessary to connect as OCI_SYSDBA. I saw some CREATE USER statements in schema manager, so this param is just in case.

Fixed error handling in OCI8Connection. When oci_connect failed, $_dbh is boolean, so cannot be used in oci_error();
…me like OCI_NO_AUTO_COMMIT - therefor any changes that are made without transaction are rolled back.

Now the OCI8 Driver act like any other driver - commit after execute by default, and starting transaction when invoked $conn->beginTransaction()
@beberlei
Copy link
Member

can you explain why you remove the autocommit of oci8? from my understanding it shouldn't produce any problems and is the preferred oracle way.

Isn't charset already included? And what does OCI_SYSDBA do?

@beberlei
Copy link
Member

I am not sure enough of the changes, can you open up issues on www.doctrine-project.org/jira for each of the different changes? That way i have a better way to analyze them.

@milokmet
Copy link
Contributor Author

milokmet commented Nov 1, 2010

Hi Benjamin,

  1. OCI_SYSDBA - you are right, I think, it is not necessary. I just tought that only SYSDBA is able to create users, but there is also a permission, that can be granted to a default user.
  2. Charset really isn't already included. Just give a try and set some. Charset is an extra parameter in oci_connect not a part of a DSN string like in PDO(_OCI).
  3. Transactions. I didn't remove the autocommit of oci8 in my branch. I've add it. I remove the obsolete OCI_DEFAULT from the OCI8Statement->execute(). In php manual is written that OCI_DEFAULT means OCI_NO_AUTO_COMMIT - which means, you need to commit everything. In doctrine DBAL there are supported more RDBMS, which default behavior is commiting everything, so it is necessary to be compatible with them.

For a better understanding:

  1. http://gist.github.com/658917 - this creates user on PostgreSQL, MySQL... but not in Oracle with OCI8.
  2. http://gist.github.com/658921 - with transactions it creates user also in Oracle.

So If I want to use some extension/bundle/plugin that was written on *SQL, I will not be able to use it on Oracle through OCI8.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants