Merge pull request phalcon#1941 from sjinks/issue-886

Make Phalcon\Mvc\Model::findFirst() work when phqlLiterals is false
NumbDai · Jan 31, 2014 · 654ec0f · 654ec0f
2 parents 65e20f7 + 70399d9
commit 654ec0f
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 65 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -237,7 +237,8 @@
    - Phalcon\Mvc\Model\Valdator now implements Phalcon\Mvc\Model\ValidatorInterface (#1852)
    - Phalcon\Mvc\View\Engine now implements Phalcon\Mvc\View\EngineInterface (#1852)
    - Added support for default parameters in Router/URL generation (#1078)
-   - Phalcon\Mvc\Model\Validator::getOption()/getOptions()/isSetOption() methods are now public
+   - Phalcon\Mvc\Model\Validator::getOption()/getOptions()/isSetOption() methods are now public (#1904)
+   - Phalcon\Mvc\Model::findFirst() now works when phqlLiterals is false (#886)
  - Phalcon\Paginator:
    - Phalcon\Paginator\Adapter\Model returns correct results even when page number is incorrect (#1654)
    - Optimized Phalcon\Paginator\Adapter\QueryBuilder (#1632)

diff --git a/ext/mvc/model.c b/ext/mvc/model.c
@@ -1370,8 +1370,8 @@ PHP_METHOD(Phalcon_Mvc_Model, find){
 PHP_METHOD(Phalcon_Mvc_Model, findFirst){
 
 	zval *parameters = NULL, *model_name, *params = NULL, *builder;
-	zval *z_one, *query, *bind_params = NULL, *bind_types = NULL, *cache;
-	zval *unique;
+	zval *query, *bind_params = NULL, *bind_types = NULL, *cache;
+	zval *unique, *index, tmp = zval_used_for_init;
 
 	PHALCON_MM_GROW();
 
@@ -1388,7 +1388,7 @@ PHP_METHOD(Phalcon_Mvc_Model, findFirst){
 		PHALCON_INIT_VAR(params);
 		array_init(params);
 		if (Z_TYPE_P(parameters) != IS_NULL) {
-			phalcon_array_append(&params, parameters, PH_SEPARATE);
+			phalcon_array_append(&params, parameters, 0);
 		}
 	} else {
 		PHALCON_CPY_WRT(params, parameters);
@@ -1403,37 +1403,58 @@ PHP_METHOD(Phalcon_Mvc_Model, findFirst){
 
 	phalcon_call_method_p1_noret(builder, "from", model_name);
 
-	/** 
-	 * We only want the first record
-	 */
-	z_one = PHALCON_GLOBAL(z_one);
-	phalcon_call_method_p1_noret(builder, "limit", z_one);
-
-	PHALCON_INIT_VAR(query);
-	phalcon_call_method(query, builder, "getquery");
-
 	/** 
 	 * Check for bind parameters
 	 */
-	PHALCON_INIT_VAR(bind_params);
-
-	PHALCON_INIT_VAR(bind_types);
-	if (phalcon_array_isset_string(params, SS("bind"))) {
-
-		PHALCON_OBS_NVAR(bind_params);
-		phalcon_array_fetch_string(&bind_params, params, SL("bind"), PH_NOISY);
-		if (phalcon_array_isset_string(params, SS("bindTypes"))) {
-			PHALCON_OBS_NVAR(bind_types);
-			phalcon_array_fetch_string(&bind_types, params, SL("bindTypes"), PH_NOISY);
+	PHALCON_OBS_VAR(bind_params);
+	PHALCON_OBS_VAR(bind_types);
+	if (phalcon_array_isset_string_fetch(&bind_params, params, SS("bind"))) {
+		Z_ADDREF_P(bind_params);
+		SEPARATE_ZVAL(&bind_params);
+		if (Z_TYPE_P(bind_params) != IS_ARRAY) {
+			zval_dtor(bind_params);
+			array_init_size(bind_params, 1);
+		}
+
+		if (phalcon_array_isset_string_fetch(&bind_types, params, SS("bindTypes"))) {
+			Z_ADDREF_P(bind_types);
+			SEPARATE_ZVAL(&bind_types);
+			if (Z_TYPE_P(bind_types) != IS_ARRAY) {
+				zval_dtor(bind_types);
+				array_init_size(bind_types, 1);
+			}
+		}
+		else {
+			MAKE_STD_ZVAL(bind_types);
+			array_init_size(bind_types, 1);
 		}
 	}
-
+	else {
+		MAKE_STD_ZVAL(bind_params);
+		MAKE_STD_ZVAL(bind_types);
+		array_init_size(bind_params, 1);
+		array_init_size(bind_types, 1);
+	}
+
+	ZVAL_LONG(&tmp, zend_hash_num_elements(Z_ARRVAL_P(bind_params)) + 1);
+	PHALCON_INIT_VAR(index);
+	PHALCON_CONCAT_SV(index, "?", &tmp);
+
+	/**
+	 * We only want the first record
+	 */
+	phalcon_call_method_p1_noret(builder, "limit", index);
+
+	PHALCON_INIT_VAR(query);
+	phalcon_call_method(query, builder, "getquery");
+
+	add_index_long(bind_params, Z_LVAL(tmp), 1);
+	add_index_long(bind_types, Z_LVAL(tmp), 1 /* BIND_PARAM_INT */);
+
 	/** 
 	 * Pass the cache options to the query
 	 */
-	if (phalcon_array_isset_string(params, SS("cache"))) {
-		PHALCON_OBS_VAR(cache);
-		phalcon_array_fetch_string(&cache, params, SL("cache"), PH_NOISY);
+	if (phalcon_array_isset_string_fetch(&cache, params, SS("cache"))) {
 		phalcon_call_method_p1_noret(query, "cache", cache);
 	}
 
@@ -1447,7 +1468,7 @@ PHP_METHOD(Phalcon_Mvc_Model, findFirst){
 	/** 
 	 * Execute the query passing the bind-params and casting-types
 	 */
-	phalcon_call_method_p2(return_value, query, "execute", bind_params, bind_types);
+	phalcon_return_call_method_p2(query, "execute", bind_params, bind_types);
 	RETURN_MM();
 }
 

diff --git a/ext/mvc/model/query.c b/ext/mvc/model/query.c
@@ -3967,7 +3967,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Query, _executeSelect){
 
 	PHALCON_INIT_VAR(sql_select);
 	phalcon_call_method_p1(sql_select, dialect, "select", intermediate);
-	
+
 	/** 
 	 * Replace the placeholders
 	 */
@@ -3986,9 +3986,9 @@ PHP_METHOD(Phalcon_Mvc_Model_Query, _executeSelect){
 			if (Z_TYPE_P(wildcard) == IS_LONG) {
 				PHALCON_INIT_NVAR(string_wildcard);
 				PHALCON_CONCAT_SV(string_wildcard, ":", wildcard);
-				phalcon_array_update_zval(&processed, string_wildcard, &value, PH_COPY | PH_SEPARATE);
+				phalcon_array_update_zval(&processed, string_wildcard, &value, PH_COPY);
 			} else {
-				phalcon_array_update_zval(&processed, wildcard, &value, PH_COPY | PH_SEPARATE);
+				phalcon_array_update_zval(&processed, wildcard, &value, PH_COPY);
 			}
 
 			zend_hash_move_forward_ex(ah5, &hp5);
@@ -4965,7 +4965,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Query, execute){
 	} else {
 		PHALCON_CPY_WRT(merged_params, bind_params);
 	}
-	
+
 	/** 
 	 * Check for default bind types and merge them with the passed ones
 	 */

diff --git a/ext/mvc/model/query/builder.c b/ext/mvc/model/query/builder.c
@@ -324,8 +324,6 @@ PHP_METHOD(Phalcon_Mvc_Model_Query_Builder, __construct){
 			if (Z_TYPE_P(limit_clause) == IS_ARRAY
 				&& phalcon_array_isset_long_fetch(&limit, limit_clause, 0)
 				&& phalcon_array_isset_long_fetch(&offset, limit_clause, 1)
-				&& phalcon_is_numeric(limit)
-				&& phalcon_is_numeric(offset)
 			) {
 				phalcon_update_property_this(this_ptr, SL("_limit"), limit TSRMLS_CC);
 				phalcon_update_property_this(this_ptr, SL("_offset"), offset TSRMLS_CC);
@@ -1291,14 +1289,10 @@ PHP_METHOD(Phalcon_Mvc_Model_Query_Builder, limit){
 
 	phalcon_fetch_params(0, 1, 1, &limit, &offset);
 
-	if (phalcon_is_numeric(limit)) {
-		phalcon_update_property_this(this_ptr, SL("_limit"), limit TSRMLS_CC);
-	}
+	phalcon_update_property_this(this_ptr, SL("_limit"), limit TSRMLS_CC);
 
-	if (offset && Z_TYPE_P(offset) != IS_NULL) {
-		if (phalcon_is_numeric(offset)) {
-			phalcon_update_property_this(this_ptr, SL("_offset"), offset TSRMLS_CC);
-		}
+	if (offset) {
+		phalcon_update_property_this(this_ptr, SL("_offset"), offset TSRMLS_CC);
 	}
 
 	RETURN_THISW();
@@ -1399,7 +1393,6 @@ PHP_METHOD(Phalcon_Mvc_Model_Query_Builder, getPhql){
 	zval *join_type = NULL, *group, *group_items, *group_item = NULL;
 	zval *escaped_item = NULL, *joined_items = NULL, *having, *order;
 	zval *order_items, *order_item = NULL, *limit, *number;
-	zval *offset = NULL;
 	HashTable *ah0, *ah1, *ah2, *ah3, *ah4, *ah5;
 	HashPosition hp0, hp1, hp2, hp3, hp4, hp5;
 	zval **hd;
@@ -1822,38 +1815,26 @@ PHP_METHOD(Phalcon_Mvc_Model_Query_Builder, getPhql){
 	/** 
 	 * Process limit parameters
 	 */
-	PHALCON_OBS_VAR(limit);
-	phalcon_read_property_this(&limit, this_ptr, SL("_limit"), PH_NOISY_CC);
+	limit = phalcon_fetch_nproperty_this(this_ptr, SL("_limit"), PH_NOISY_CC);
 	if (Z_TYPE_P(limit) != IS_NULL) {
 		if (Z_TYPE_P(limit) == IS_ARRAY) { 
+			zval *offset;
 
 			PHALCON_OBS_VAR(number);
 			phalcon_array_fetch_string(&number, limit, SL("number"), PH_NOISY);
-			if (phalcon_array_isset_string(limit, SS("offset"))) {
-
-				PHALCON_OBS_VAR(offset);
-				phalcon_array_fetch_string(&offset, limit, SL("offset"), PH_NOISY);
-				if (phalcon_is_numeric(offset)) {
-					PHALCON_SCONCAT_SVSV(phql, " LIMIT ", number, " OFFSET ", offset);
-				} else {
-					PHALCON_SCONCAT_SVS(phql, " LIMIT ", number, " OFFSET 0");
-				}
+			if (phalcon_array_isset_string_fetch(&offset, limit, SS("offset")) && Z_TYPE_P(offset) != IS_NULL) {
+				PHALCON_SCONCAT_SVSV(phql, " LIMIT ", number, " OFFSET ", offset);
 			} else {
 				PHALCON_SCONCAT_SV(phql, " LIMIT ", number);
 			}
 		} else {
-			if (phalcon_is_numeric(limit)) {
-				PHALCON_SCONCAT_SV(phql, " LIMIT ", limit);
-
-				PHALCON_OBS_NVAR(offset);
-				phalcon_read_property_this(&offset, this_ptr, SL("_offset"), PH_NOISY_CC);
-				if (Z_TYPE_P(offset) != IS_NULL) {
-					if (phalcon_is_numeric(offset)) {
-						PHALCON_SCONCAT_SV(phql, " OFFSET ", offset);
-					} else {
-						phalcon_concat_self_str(&phql, SL(" OFFSET 0") TSRMLS_CC);
-					}
-				}
+			zval *offset;
+
+			PHALCON_SCONCAT_SV(phql, " LIMIT ", limit);
+
+			offset = phalcon_fetch_nproperty_this(this_ptr, SL("_offset"), PH_NOISY_CC);
+			if (Z_TYPE_P(offset) != IS_NULL) {
+				PHALCON_SCONCAT_SV(phql, " OFFSET ", offset);
 			}
 		}
 	}

diff --git a/unit-tests/ModelsTest.php b/unit-tests/ModelsTest.php
@@ -37,6 +37,13 @@ public function __destruct()
 		spl_autoload_unregister(array($this, 'modelsAutoloader'));
 	}
 
+	public function tearDown()
+	{
+		Phalcon\Mvc\Model::setup(array(
+			'phqlLiterals' => true,
+		));
+	}
+
 	public function modelsAutoloader($className)
 	{
 		if (file_exists('unit-tests/models/'.$className.'.php')) {
@@ -98,6 +105,7 @@ public function testModelsMysql()
 		$this->_executeTestsRenamed($di);
 
 		$this->issue1534($di);
+		$this->issue886($di);
 	}
 
 	public function testModelsPostgresql()
@@ -115,6 +123,8 @@ public function testModelsPostgresql()
 
 		$this->_executeTestsNormal($di);
 		$this->_executeTestsRenamed($di);
+
+		$this->issue886($di);
 	}
 
 	public function testModelsSqlite()
@@ -132,6 +142,8 @@ public function testModelsSqlite()
 
 		$this->_executeTestsNormal($di);
 		$this->_executeTestsRenamed($di);
+
+		$this->issue886($di);
 	}
 
 	protected function issue1534($di)
@@ -191,6 +203,19 @@ protected function issue1534($di)
 		$this->assertTrue($db->delete('issue_1534'));
 	}
 
+	protected function issue886($di)
+	{
+		$this->_prepareDb($di->getShared('db'));
+
+		Phalcon\Mvc\Model::setup(array(
+			'phqlLiterals' => false,
+		));
+
+		$people = People::findFirst();
+		$this->assertTrue(is_object($people));
+		$this->assertEquals(get_class($people), 'People');
+	}
+
 	protected function _executeTestsNormal($di){
 
 		$this->_prepareDb($di->getShared('db'));