[Bug #77120] Handle OCI_SUCCESS_WITH_INFO returned from OCISessionBegin

[morozov]

Create a test user:

CREATE PROFILE test_profile LIMIT PASSWORD_LIFE_TIME 1/86400 PASSWORD_GRACE_TIME 7;
CREATE USER bob IDENTIFIED BY password PROFILE test_profile;
GRANT CREATE SESSION TO bob;

Run the script:

$conn = new PDO('oci:dbname=(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=1521))(CONNECT_DATA=(SID=XE)))', 'bob', 'password');

Expected result:
The connection is successfully established. The details of the informational message are available via PDO::errorInfo().

Actual result:

PDOException: SQLSTATE[HY000]: OCISessionBegin: OCI_SUCCESS_WITH_INFO: ORA-28002: the password will expire within 7 days
 (ext/pdo_oci/oci_driver.c:685) in test.php on line 3

Change summary:

1. Proceed with connection initialization if OCI_SUCCESS_WITH_INFO is returned.
2. Do not throw an exception upon OCI_SUCCESS_WITH_INFO.

[nikic]

cc @cjbj

[morozov]

@nikic is there a way to proceed given the absence of @cjbj's feedback?

[cjbj]

I'd like to review it first.

[ramsey]

@morozov Is it possible to provide a test with this?

[morozov]

Absolutely. I just want to make sure the fix as such is acceptable.

[cjbj]

Adding a test now would expedite me reviewing it. Otherwise I have to create the test... :)

[morozov]

I can add a test by Monday.

[morozov]

@cjbj, @ramsey please see the test.

[cjbj]

@morozov Thank you. I should get time later this week to take a look.

[cjbj]

Can you also check some other SUCCESS_WITH_INFO cases such as PL/SQL compilation failures? Check them for various PDO::ATTR_ERRMODE values. (And verify the old & new behaviors). Thank you!

[morozov]

Can you also check some other SUCCESS_WITH_INFO cases such as PL/SQL compilation failures? Check them for various PDO::ATTR_ERRMODE values.

What would be the best way to reproduce this? I tried a couple of examples I found online (e.g. this) in PhpStorm but it didn't trigger the expected error.

[morozov]

I manages to reproduce the compilation error with the following:

function triggerCompilationError(PDO $conn): void {
    $conn->exec(<<<'SQL'
CREATE OR REPLACE FUNCTION BUG77120(INT A) RETURN INT
AS
BEGIN
    RETURN 0;
END;
SQL
    );
}

triggerCompilationError($conn);

$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
triggerCompilationError($conn);

Which produces the following:

Warning: PDO::exec(): SQLSTATE[HY000]: General error: 24344 OCIStmtExecute: OCI_SUCCESS_WITH_INFO: ORA-24344: success with compilation error
(ext/pdo_oci/oci_driver.c:340) in ext/pdo_oci/tests/oci_success_with_info.php on line 41

Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: 24344 OCIStmtExecute: OCI_SUCCESS_WITH_INFO: ORA-24344: success with compilation error
(ext/pdo_oci/oci_driver.c:340) in ext/pdo_oci/tests/oci_success_with_info.php:41

I assume the error handling still exists in this scenario but it's out of the scope of the original issue. Statement error handling can be configured at runtime while connection error handling cannot.

[cjbj]

I assume the error handling still exists in this scenario but it's out of the scope of the original issue. Statement error handling can be configured at runtime while connection error handling cannot.

The PR changes _oci_error() which is called in oci_statement.c so it seems wiser to check that any existing statement error handling isn't borked.

You could use create or replace procedure p() as begin null; end; to create a compilation error.

[morozov]

The PR changes _oci_error() which is called in oci_statement.c so it seems wiser to check that any existing statement error handling isn't borked.

It wasn't borked, it worked as previously, see #6857 (comment).

Please see the update. I modified handling OCI_SUCCESS_WITH_INFO returned from OCIStmtExecute and updated the test.

[cjbj]

Thanks for tidying up, and for adding the extra checks to confirm the behavior.
With the trivial test changes I noted, let's get this merged.
Thanks for making PDO_OCI better.

[morozov]

@cjbj thanks for the review. Please see the update.

[morozov]

@cjbj any chance you could review this again?

[cjbj]

LGTM

@ramsey can you advise on the (new) current process for merging?

[morozov]

@ramsey what would be the next step to get this patch accepted?

[ramsey]

All good. Sorry for the delay. Thanks!

[cmb69]

This should have an entry in NEWS.

[cjbj]

This should have an entry in NEWS.

Done. Thanks all.

[ramsey]

Thanks, @cjbj!