1) The user would never pass a NULL sql:
<?
$conn = OCILogon ("bla", "bla", "bla");
$sql = NULL;
$stmt = OCIParse ($conn, $sql);
OCIExecute ($stmt, OCI_DEFAULT);
$error = OCIError ($stmt); <-- seg fault here
print_r ($error);
?>
This condition would cause OCIAttrGet to set sqltext to NULL, leading me to the presumtion...
2) That add_assoc_string() would be able to handle a NULL pointer.
It does not. So I made sure sqltext is not NULL when I send it to add_assoc_string. I've decided to send an empty string as opposed to simply omitting sqltext. This way the array returned always has the same number of elements.
3) A call to OCIError with a $conn arg or no arg would not affect anything.
Well, it did. Seg fault. I've moved the calls to OCIAttrGet into the if (statement) { } block. I should have caught this from the very beginning. My bad.
I'll conduct a few more tests. Anyone else want to help me test this?
I've attached revised patches against the latest cvs and 4.1.2.
Daniel
Markus Fischer wrote:
Have you tested this patch also when there is no statement
but only a connection or any kind of global error?
My concernc are what happens if sqltext remains a NULL
pointer?
On Wed, Mar 13, 2002 at 11:32:03AM -0800, Daniel Ceregatti wrote :This patch simply adds the original query text as the "sqltext" element
and the error offset as the "offset" element. It was made against the
latest CVS. Hopfully it'll suffice. If not, please let me know.
Daniel
Markus Fischer wrote:Of course the offset is fine. But if I were you, I wouldn't
put the asterisk into the sql statement; just provide the
offset. _This_ is the greatest flexibility you can provide
because all the information is passed unmodified to the
developer.
Serious environments need custom error handlers anyway.
Putting HTML inside or modifying error messages this way is
a bad thing [tm]. Just provide the raw components and
everyone else can decide how he presents the data to the end
user (that's the idea behind).
And, for the patch, can you please make a unified diff
against altest CVS ?
- Markus
On Tue, Mar 12, 2002 at 03:19:27PM -0800, Walter A. Boring IV wrote :I like the idea of having the sqltext in the return array, as well as
the offset. This is a very usefull tool for debugging oracle sql
queries. I believe its something that OCIError has been lacking.
Anyone that uses Oracle along with sqlplus, you get the * under the
broken portion of the query. I find it very usefull.
If there is a general rule of not putting html inside return values
for php internal functions, thats fine. But I still want an easy way of
showing this type of info, without having to create my own wrapper
function call to OCIError, just so I can highlight the broken portion of
the query.
my $0.02
Walt
On Tue, 2002-03-12 at 15:08, Daniel Ceregatti wrote:Yes. An example of what would be in the array returned by OCIError would
be:
// Given the code below...
$conn = OCILogon (bla bla bla);
$sql = "select t.foo, t.bar from table t where t.id = 1";
$stmt = OCIParse ($conn, $sql);
OCIExecute ($stmt, OCI_DEFAULT);
$error = OCIError ($stmt);
// What follows are the values of the elements of the array returned by
OCIError (presuming "bar" is an invalid column it the table):
$error["code"] = 904
$error["message"] = "ORA-00904: invalid column name"
$error["sqltext"] = "select t.foo, t.*bar from table t where t.id = 1"
$error["offset"] = 16
The PHP user then has the option of using either the "sqltext" element
directly, or using the "offset" element and the $sql variable to create
any output they see fit. I think this allows for the greatest
felxibility, IMHO.
Daniel
Markus Fischer wrote:On Tue, Mar 12, 2002 at 02:26:24PM -0800, Daniel Ceregatti wrote :How about doing what sqlplus does and simply add an asterisk at that
point? I'm ok with rolling my own inside of the script by using the
offset element. I simply think it'll do PHP users a service to have
the "sqltext" in the array in _some_ form, even if only with an
asterisk.Asterisk at what point? Specified by offset?diff -Naur php4-cvs/ext/oci8/oci8.c php4/ext/oci8/oci8.c
--- php4-cvs/ext/oci8/oci8.c Wed Mar 13 11:14:18 2002
+++ php4/ext/oci8/oci8.c Wed Mar 13 11:24:40 2002
@@ -4218,10 +4218,12 @@
zval **arg;
oci_statement *statement;
oci_connection *connection;
- text errbuf[512];
- sb4 errcode = 0;
+ text errbuf[512];
+ sb4 errcode = 0;
sword error = 0;
dvoid *errh = NULL;
+ ub2 errorofs = 0;
+ text *sqltext = NULL;
if (zend_get_parameters_ex(1, &arg) == SUCCESS) {
statement = (oci_statement *) zend_fetch_resource(arg TSRMLS_CC, -1, NULL, NULL, 1, le_stmt);
@@ -4258,10 +4260,28 @@
(ub4) sizeof(errbuf),
(ub4) OCI_HTYPE_ERROR));
+ CALL_OCI_RETURN(statement->error, OCIAttrGet(
+ (dvoid *)statement->pStmt,
+ OCI_HTYPE_STMT,
+ (text *) &sqltext,
+ (ub4 *)0,
+ OCI_ATTR_STATEMENT,
+ statement->pError));
+
+ CALL_OCI_RE TURN(statement->error, OCIAttrGet(
+ (dvoid *)statement->pStmt,
+ OCI_HTYPE_STMT,
+ (ub2 *)&errorofs,
+ (ub4 *)0,
+ OCI_ATTR_PARSE_ERROR_OFFSET,
+ statement->pError));
+
if (errcode) {
array_init(return_value);
add_assoc_long(return_value, "code", errcode);
+ add_assoc_long(return_value, "offset", errorofs);
add_assoc_string(return_value, "message", (char*) errbuf, 1);
+ add_assoc_string(return_value, "sqltext", (char *) sqltext, 1);
} else {
RETURN_FALSE;
}
--- php-4.1.2/ext/oci8/oci8.c.orig Wed Mar 13 14:51:19 2002 +++ php-4.1.2/ext/oci8/oci8.c Wed Mar 13 16:20:27 2002 @@ -4090,16 +4090,35 @@ zval **arg; oci_statement *statement; oci_connection *connection; - text errbuf[512]; - sb4 errcode = 0; + text errbuf[512]; + sb4 errcode = 0; sword error = 0; dvoid *errh = NULL; + ub2 errorofs = 0; + text *sqltext = NULL; if (zend_get_parameters_ex(1, &arg) == SUCCESS) { statement = (oci_statement *) zend_fetch_resource(arg TSRMLS_CC, -1, NULL, NULL, 1, le_stmt); if (statement) { errh = statement->pError; error = statement->error; + + CALL_OCI_RETURN(statement->error, OCIAttrGet( + (dvoid *)statement->pStmt, + OCI_HTYPE_STMT, + (text *) &sqltext, + (ub4 *)0, + OCI_ATTR_STATEMENT, + statement->pError)); + + CALL_OCI_RETURN(statement->error, OCIAttrGet( + (dvoid *)statement->pStmt, + OCI_HTYPE_STMT, + (ub2 *)&errorofs, + (ub4 *)0, + OCI_ATTR_PARSE_ERROR_OFFSET, + statement->pError)); + } else { connection = (oci_connection *) zend_fetch_resource(arg TSRMLS_CC, -1, NULL, NULL, 1, le_conn); if (connection) { @@ -4133,6 +4152,13 @@ if (errcode) { array_init(return_value); add_assoc_long(return_value, "code", errcode); + add_assoc_long(return_value, "offset", errorofs); + if (sqltext == NULL) { + sqltext = (text *) emalloc (sizeof (text)); + *sqltext = '\0'; + } + add_assoc_string(return_value, "sqltext", (char *) sqltext, 1); + efree (sqltext); add_assoc_string(return_value, "message", (char*) errbuf, 1); } else { RETURN_FALSE;diff -Naur php4-cvs/ext/oci8/oci8.c php4/ext/oci8/oci8.c --- php4-cvs/ext/oci8/oci8.c Wed Mar 13 11:14:18 2002 +++ php4/ext/oci8/oci8.c Wed Mar 13 16:32:58 2002 @@ -4218,16 +4218,35 @@ zval **arg; oci_statement *statement; oci_connection *connection; - text errbuf[512]; - sb4 errcode = 0; + text errbuf[512]; + sb4 errcode = 0; sword error = 0; dvoid *errh = NULL; + ub2 errorofs = 0; + text *sqltext = NULL; if (zend_get_parameters_ex(1, &arg) == SUCCESS) { statement = (oci_statement *) zend_fetch_resource(arg TSRMLS_CC, -1, NULL, NULL, 1, le_stmt); if (statement) { errh = statement->pError; error = statement->error; + + CALL_OCI_RETURN(statement->error, OCIAttrGet( + (dvoid *)statement->pStmt, + OCI_HTYPE_STMT, + (text *) &sqltext, + (ub4 *)0, + OCI_ATTR_STATEMENT, + statement->pError)); + + CALL_OCI_RETURN(statement->error, OCIAttrGet( + (dvoid *)statement->pStmt, + OCI_HTYPE_STMT, + (ub2 *)&errorofs, + (ub4 *)0, + OCI_ATTR_PARSE_ERROR_OFFSET, + statement->pError)); + } else { connection = (oci_connection *) zend_fetch_resource(arg TSRMLS_CC, -1, NULL, NULL, 1, le_conn); if (connection) { @@ -4261,7 +4280,13 @@ if (errcode) { array_init(return_value); add_assoc_long(return_value, "code", errcode); + add_assoc_long(return_value, "offset", errorofs); add_assoc_string(return_value, "message", (char*) errbuf, 1); + if (!sqltext) { + + sqltext = (text *) emalloc (sizeof (text)); + *sqltext = '\0'; + } + add_assoc_string(return_value, "sqltext", (char *) sqltext, 1); } else { RETURN_FALSE; }-- PHP Development Mailing List <http://www.php.net/> To unsubscribe, visit: http://www.php.net/unsub.php
