From c2df2f3a45aeaabeae971c7c91a062033baf68e9 Mon Sep 17 00:00:00 2001 From: ucorina Date: Mon, 2 Apr 2012 15:36:50 +0000 Subject: [PATCH] Issue 1819: CUBRID prepare statement error git-svn-id: http://xe-core.googlecode.com/svn/branches/1.5.0@10525 201d5d3c-b55e-5fd7-737f-ddc643e51545 --- classes/db/DBCubrid.class.php | 2 + classes/db/DBMssql.class.php | 25 ++++----- classes/db/DBMysqli.class.php | 10 +++- classes/db/queryparts/Query.class.php | 54 ++++++++++--------- .../queryparts/condition/Condition.class.php | 22 +++++--- .../condition/ConditionWithArgument.class.php | 29 ++++++---- .../xml/xmlquery/argument/Argument.class.php | 11 +++- .../argument/ConditionArgument.class.php | 39 +++++++++----- .../db/xml_query/cubrid/CubridSelectTest.php | 40 ++++++++++++++ .../cubrid/data/resource.getLatestItem.xml | 40 ++++++++++++++ 10 files changed, 201 insertions(+), 71 deletions(-) create mode 100644 tests/classes/db/db/xml_query/cubrid/data/resource.getLatestItem.xml diff --git a/classes/db/DBCubrid.class.php b/classes/db/DBCubrid.class.php index 24d67628a..c3025f91d 100644 --- a/classes/db/DBCubrid.class.php +++ b/classes/db/DBCubrid.class.php @@ -179,6 +179,8 @@ { $value = $param->getUnescapedValue(); $type = $param->getType(); + + if($param->isColumnName()) continue; switch($type) { diff --git a/classes/db/DBMssql.class.php b/classes/db/DBMssql.class.php index 2437e7124..63edb8af8 100644 --- a/classes/db/DBMssql.class.php +++ b/classes/db/DBMssql.class.php @@ -135,18 +135,19 @@ if(count($this->param)){ foreach($this->param as $k => $o){ - if($o->getType() == 'number'){ - $value = $o->getUnescapedValue(); - if(is_array($value)) $_param = array_merge($_param, $value); - else $_param[] = $o->getUnescapedValue(); - }else{ - $value = $o->getUnescapedValue(); - if(is_array($value)) { - foreach($value as $v) - $_param[] = array($v, SQLSRV_PARAM_IN, SQLSRV_PHPTYPE_STRING('utf-8')); - } - else $_param[] = array($value, SQLSRV_PARAM_IN, SQLSRV_PHPTYPE_STRING('utf-8')); - } + if($o->isColumnName()) continue; + if($o->getType() == 'number'){ + $value = $o->getUnescapedValue(); + if(is_array($value)) $_param = array_merge($_param, $value); + else $_param[] = $o->getUnescapedValue(); + }else{ + $value = $o->getUnescapedValue(); + if(is_array($value)) { + foreach($value as $v) + $_param[] = array($v, SQLSRV_PARAM_IN, SQLSRV_PHPTYPE_STRING('utf-8')); + } + else $_param[] = array($value, SQLSRV_PARAM_IN, SQLSRV_PHPTYPE_STRING('utf-8')); + } } } diff --git a/classes/db/DBMysqli.class.php b/classes/db/DBMysqli.class.php index ea62a8505..1c53af938 100644 --- a/classes/db/DBMysqli.class.php +++ b/classes/db/DBMysqli.class.php @@ -112,14 +112,16 @@ } // 2. Bind parameters - call_user_func_array('mysqli_stmt_bind_param',$args); + $status = call_user_func_array('mysqli_stmt_bind_param',$args); + if(!$status) + $this->setError(-1, "Invalid arguments: $query" . mysqli_error($connection) . PHP_EOL . print_r($args, true)); } // 3. Execute query $status = mysqli_stmt_execute($stmt); if(!$status) - $this->setError(-1, "Prepared statement failed: $query"); + $this->setError(-1, "Prepared statement failed: $query" . mysqli_error($connection) . PHP_EOL . print_r($args, true)); // Return stmt for other processing - like retrieving resultset (_fetch) return $stmt; @@ -146,6 +148,10 @@ foreach($this->param as $k => $o){ $value = $o->getUnescapedValue(); $type = $o->getType(); + + // Skip column names -> this should be concatenated to query string + if($o->isColumnName()) continue; + switch($type) { case 'number' : diff --git a/classes/db/queryparts/Query.class.php b/classes/db/queryparts/Query.class.php index 88d5d41fa..ad052294f 100644 --- a/classes/db/queryparts/Query.class.php +++ b/classes/db/queryparts/Query.class.php @@ -223,34 +223,36 @@ function getWhereString($with_values = true, $with_optimization = true){ $where = ''; - $condition_count = 0; - - foreach($this->conditions as $conditionGroup){ - if($condition_count === 0){ - $conditionGroup->setPipe(""); - } - $condition_string = $conditionGroup->toString($with_values); - $where .= $condition_string; - $condition_count++; - } - - if($with_optimization && - (strstr($this->getOrderByString(), 'list_order') || strstr($this->getOrderByString(), 'update_order'))){ - - if($condition_count !== 0) $where = '(' . $where .') '; - - foreach($this->orderby as $order){ - $colName = $order->getColumnName(); - if(strstr($colName, 'list_order') || strstr($colName, 'update_order')){ - $opt_condition = new ConditionWithoutArgument($colName, 2100000000, 'less', 'and'); - if ($condition_count === 0) $opt_condition->setPipe(""); - $where .= $opt_condition->toString($with_values).' '; - $condition_count++; + $condition_count = 0; + + foreach ($this->conditions as $conditionGroup) { + if ($condition_count === 0) { + $conditionGroup->setPipe(""); } - } + $condition_string = $conditionGroup->toString($with_values); + $where .= $condition_string; + $condition_count++; } - - return trim($where); + + if ($with_optimization && + (strstr($this->getOrderByString(), 'list_order') || strstr($this->getOrderByString(), 'update_order'))) { + + if ($condition_count !== 0) + $where = '(' . $where . ') '; + + foreach ($this->orderby as $order) { + $colName = $order->getColumnName(); + if (strstr($colName, 'list_order') || strstr($colName, 'update_order')) { + $opt_condition = new ConditionWithoutArgument($colName, 2100000000, 'less', 'and'); + if ($condition_count === 0) + $opt_condition->setPipe(""); + $where .= $opt_condition->toString($with_values) . ' '; + $condition_count++; + } + } + } + + return trim($where); } function getGroupByString(){ diff --git a/classes/db/queryparts/condition/Condition.class.php b/classes/db/queryparts/condition/Condition.class.php index 5fe11adf3..c8e55d0aa 100644 --- a/classes/db/queryparts/condition/Condition.class.php +++ b/classes/db/queryparts/condition/Condition.class.php @@ -24,13 +24,21 @@ } function toString($withValue = true){ - if(!isset($this->_value_to_string)){ - if(!$this->show()) { $this->_value_to_string = ''; } - else if($withValue) - $this->_value_to_string = $this->toStringWithValue(); - else $this->_value_to_string = $this->toStringWithoutValue(); - } - return $this->_value_to_string; + if (!isset($this->_value_to_string)) { + if (!$this->show()) + { + $this->_value_to_string = ''; + } + else if ($withValue) + { + $this->_value_to_string = $this->toStringWithValue(); + } + else + { + $this->_value_to_string = $this->toStringWithoutValue(); + } + } + return $this->_value_to_string; } function toStringWithoutValue(){ diff --git a/classes/db/queryparts/condition/ConditionWithArgument.class.php b/classes/db/queryparts/condition/ConditionWithArgument.class.php index 5a00153f3..5badb729f 100644 --- a/classes/db/queryparts/condition/ConditionWithArgument.class.php +++ b/classes/db/queryparts/condition/ConditionWithArgument.class.php @@ -14,16 +14,27 @@ } function toStringWithoutValue(){ - $value = $this->argument->getUnescapedValue(); + $value = $this->argument->getUnescapedValue(); - if(is_array($value)){ - $q = ''; - foreach ($value as $v) $q .= '?,'; - if($q !== '') $q = substr($q, 0, -1); - $q = '(' . $q . ')'; - } - else $q = '?'; - return $this->pipe . ' ' . $this->getConditionPart($q); + if(is_array($value)){ + $q = ''; + foreach ($value as $v) $q .= '?,'; + if($q !== '') $q = substr($q, 0, -1); + $q = '(' . $q . ')'; + } + else + { + // Prepared statements: column names should not be sent as query arguments, but instead concatenated to query string + if($this->argument->isColumnName()) + { + $q = $value; + } + else + { + $q = '?'; + } + } + return $this->pipe . ' ' . $this->getConditionPart($q); } function show(){ diff --git a/classes/xml/xmlquery/argument/Argument.class.php b/classes/xml/xmlquery/argument/Argument.class.php index 49b92d3cb..d44fe36dd 100644 --- a/classes/xml/xmlquery/argument/Argument.class.php +++ b/classes/xml/xmlquery/argument/Argument.class.php @@ -20,7 +20,9 @@ class Argument { function getType() { if (isset($this->type)) + { return $this->type; + } if (is_string($this->value)) return 'column_name'; return 'number'; @@ -29,7 +31,7 @@ class Argument { function setColumnType($value) { $this->type = $value; } - + function setColumnOperation($operation) { $this->column_operation = $operation; } @@ -113,6 +115,13 @@ class Argument { function isValid() { return $this->isValid; } + + function isColumnName(){ + $type = $this->getType(); + if($type == 'column_name') return true; + if($type == 'number' && !is_numeric($this->value) && $this->uses_default_value) return true; + return false; + } function getErrorMessage() { return $this->errorMessage; diff --git a/classes/xml/xmlquery/argument/ConditionArgument.class.php b/classes/xml/xmlquery/argument/ConditionArgument.class.php index 4b654f417..b4b9bfe7b 100644 --- a/classes/xml/xmlquery/argument/ConditionArgument.class.php +++ b/classes/xml/xmlquery/argument/ConditionArgument.class.php @@ -63,22 +63,33 @@ } } - /** - * Since ConditionArgument is used in WHERE clause, - * where the argument value is compared to a table column, - * it is assumed that all arguments have type. There are cases though - * where the column does not have any type - if it was removed from - * the XML schema for example - see the is_secret column in xe_documents table. - * In this case, the column type is retrieved according to argument - * value type (using the PHP function is_numeric). - * - * @return type string - */ - function getType(){ - return $this->type ? $this->type : (!is_numeric($this->value) ? "varchar" : ""); + /** + * Since ConditionArgument is used in WHERE clause, + * where the argument value is compared to a table column, + * it is assumed that all arguments have type. There are cases though + * where the column does not have any type - if it was removed from + * the XML schema for example - see the is_secret column in xe_documents table. + * In this case, the column type is retrieved according to argument + * value type (using the PHP function is_numeric). + * + * @return type string + */ + function getType(){ + if($this->type) + { + return $this->type; + } + else if(!is_numeric($this->value)) + { + return 'varchar'; + } + else + { + return ''; + } } - function setColumnType($column_type){ + function setColumnType($column_type){ if(!isset($this->value)) return; if($column_type === '') return; diff --git a/tests/classes/db/db/xml_query/cubrid/CubridSelectTest.php b/tests/classes/db/db/xml_query/cubrid/CubridSelectTest.php index 31c1f1f78..aef54a3dd 100644 --- a/tests/classes/db/db/xml_query/cubrid/CubridSelectTest.php +++ b/tests/classes/db/db/xml_query/cubrid/CubridSelectTest.php @@ -400,5 +400,45 @@ define('__CUBRID_VERSION__', '8.4.1'); $this->_test($xml_file, $argsString, $expected); } + + + function test_resource_getLatestItem(){ + $xml_file = _TEST_PATH_ . "db/xml_query/cubrid/data/resource.getLatestItem.xml"; + $expected = 'SELECT "package"."module_srl" as "module_srl" + , "package"."status" as "status" + , "package"."category_srl" as "category_srl" + , "package"."member_srl" as "member_srl" + , "package"."package_srl" as "package_srl" + , "package"."path" as "path" + , "package"."license" as "license" + , "package"."title" as "title" + , "package"."homepage" as "homepage" + , "package"."description" as "package_description" + , "package"."voter" as "package_voter" + , "package"."voted" as "package_voted" + , "package"."downloaded" as "package_downloaded" + , "package"."regdate" as "package_regdate" + , "package"."last_update" as "package_last_update" + , "member"."nick_name" as "nick_name" + , "member"."user_id" as "user_id" + , "item"."item_srl" as "item_srl" + , "item"."document_srl" as "document_srl" + , "item"."file_srl" as "item_file_srl" + , "item"."screenshot_url" as "item_screenshot_url" + , "item"."version" as "item_version" + , "item"."voter" as "item_voter" + , "item"."voted" as "item_voted" + , "item"."downloaded" as "item_downloaded" + , "item"."regdate" as "item_regdate" + FROM "xe_resource_packages" as "package" + , "xe_member" as "member" + , "xe_resource_items" as "item" + WHERE "package"."package_srl" = ? + and "package"."member_srl" = "member"."member_srl" + and "item"."item_srl" = "package"."latest_item_srl"'; + $argsString = '$args->package_srl = 18325662;'; + $expectedArgs = array(18325662); + $this->_testPreparedQuery($xml_file, $argsString, $expected, 'getSelectSql', $expectedArgs); + } } \ No newline at end of file diff --git a/tests/classes/db/db/xml_query/cubrid/data/resource.getLatestItem.xml b/tests/classes/db/db/xml_query/cubrid/data/resource.getLatestItem.xml new file mode 100644 index 000000000..f6fa06e83 --- /dev/null +++ b/tests/classes/db/db/xml_query/cubrid/data/resource.getLatestItem.xml @@ -0,0 +1,40 @@ + + + +
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +