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

[BUG]: PHQL; Undefined index: ... in phalcon/Mvc/Model/Resultset/Complex.zep #14657

Closed
Deathamns opened this issue Dec 24, 2019 · 12 comments · Fixed by #15029
Closed

[BUG]: PHQL; Undefined index: ... in phalcon/Mvc/Model/Resultset/Complex.zep #14657

Deathamns opened this issue Dec 24, 2019 · 12 comments · Fixed by #15029
Assignees
Labels
bug A bug report status: low Low

Comments

@Deathamns
Copy link
Contributor

Describe the bug
Can't run PHQL query when selecting columns from a table with asterisk, and specific table aliased columns from some other table. No problem in v3.4.5.
Might be related to #14488.

PHP Notice: Undefined index: obj_name in phalcon/Mvc/Model/Resultset/Complex.zep on line 224

To Reproduce

<?php
class Objects extends \Phalcon\Mvc\Model
{
	public $obj_id;
	public $obj_name;
	public $obj_type;
}

class ObjectDetails extends \Phalcon\Mvc\Model
{
	public $obj_id;
	public $details;
}

$di = new \Phalcon\Di\FactoryDefault;

$di['db'] = new \Phalcon\Db\Adapter\Pdo\Mysql([
	'dbname'   => 'test',
	'username' => 'test',
	'password' => 'test',
]);

try {
	$app = new \Phalcon\Mvc\Application($di);
	$res = $app->modelsManager->executeQuery(
		'SELECT o.obj_name, obj_type, od.* FROM Objects AS o ' .
		'LEFT JOIN ObjectDetails AS od ON od.obj_id = o.obj_id ' .
		'WHERE o.obj_id  = :obj_id:',
		['obj_id' => 1]
	)->getFirst();

	echo $res->obj_name;
	// $res->od is available, and if there are values, it is correctly filled
} catch ( Exception $ex ) {
	print_r($ex->getMessage());
}

Expected behavior
To be able to run the query and access the table aliased columns.

Details

  • Phalcon version: 4.0.0, Dec 21 2019 18:42:18, Zephir Version 0.12.15-814db50
  • PHP Version: PHP 7.4.1 (cli) (built: Dec 17 2019 19:23:59) ( NTS Visual C++ 2017 x64 )
  • Database, table schema:
CREATE TABLE IF NOT EXISTS `objects` (
  `obj_id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
  `obj_name` VARCHAR(20) NOT NULL,
  `obj_type` TINYINT(3) UNSIGNED NOT NULL,
  PRIMARY KEY (`obj_id`)
) ENGINE=InnoDB;

CREATE TABLE IF NOT EXISTS `object_details` (
  `obj_id` INT(10) UNSIGNED NOT NULL,
  `details` VARCHAR(225) NOT NULL,
  PRIMARY KEY (`obj_id`)
) ENGINE=InnoDB;

INSERT INTO `objects` (`obj_id`, `obj_name`, `obj_type`) VALUES (1, 'test1', 2);
@niden niden added bug A bug report status: unverified Unverified labels Dec 24, 2019
@ruudboon ruudboon added the 4.0.1 label Jan 2, 2020
@ruudboon
Copy link
Member

ruudboon commented Jan 2, 2020

Let's investigate this bug in the current sprint. If we can fix it easily in Zephir we do it in 4.0.x if work is required in the PHQL parser we move this to 4.1.x

@ruudboon ruudboon added status: low Low and removed status: unverified Unverified labels Jan 7, 2020
@ruudboon
Copy link
Member

ruudboon commented Jan 7, 2020

Looks like this issue still occurs after the #14566 fix. Let's have a chat @niden if we can fix it in this iteration or have to reschedule this.

@niden
Copy link
Member

niden commented Jan 11, 2020

I just spent 5 hours on this and not getting close to finding it. From previous iterations of this bug such as #14488 (because they are related) I found that there was a change made to the columnMap and how aliases are calculated (AS x). I have no idea which commit it is that did this but it happened clearly in v4.

For the above example we could access

$res->obj_name

i.e. the results would come in ignoring the AS o from the query. Right now it comes back as:

$res->o_obj_name

If we remove the AS o from the Objects table this works:

$res->obj_name

@niden
Copy link
Member

niden commented Jan 11, 2020

Honestly it will be easier to tell people that if you declare an alias, then it will prefix your fields (in the docs) but I am not 100% sure that this is the case and I have not been able to find the commit (or the place in the code) that does this....

@ruudboon
Copy link
Member

Let's add the comment to our docs for now. We can close this or keep it open and move it to 4.1 and have a fresh look at this when we look at the PHQL fixes as well.

@ruudboon ruudboon added 4.0.2 and removed 4.0.1 labels Jan 14, 2020
@shurastik
Copy link

Honestly it will be easier to tell people that if you declare an alias, then it will prefix your fields (in the docs) but I am not 100% sure that this is the case and I have not been able to find the commit (or the place in the code) that does this....

But how about legacy applications? If this behavior becomes a BC-break, a ton of code should be rewritten.

@ruudboon
Copy link
Member

@sergeyklay Can you shine your light on this one?

@sergeyklay
Copy link
Contributor

I'll try to sort out

@kirill-voronov
Copy link

This also crashes Phalcon\Mvc\Model\Query\Builder result set, for example:

// ... build some query into $builder
$items = $builder->getQuery()->execute();
foreach ($items as $item) { // this line fails
}

@sergeyklay
Copy link
Contributor

sergeyklay commented May 6, 2020

Btw, there is a typo in the SQL (ambiguous column name: obj_type):

SELECT
    o.obj_name,
-   obj_type,
+   o.obj_type,
    od.*
FROM
    Objects AS o
LEFT JOIN ObjectDetails AS od ON
    od.obj_id = o.obj_id
WHERE
    o.obj_id = 1

@sergeyklay
Copy link
Contributor

sergeyklay commented May 7, 2020

Found the bad commit using git bisect: e851f48 (#13663)

Code to reproduce:

use Phalcon\Db\Adapter\Pdo\Sqlite;
use Phalcon\Di;
use Phalcon\Di\FactoryDefault;
use Phalcon\Mvc\Application;
use Phalcon\Mvc\Model;

Di::setDefault($container = new FactoryDefault());

$container->setShared(
    'db',
    new Sqlite(
        ['dbname' => '/tmp/test.sqlite']
    )
);

class Objects extends Model
{
    public $obj_id;
    public $obj_name;
    public $obj_type;
}

class ObjectDetails extends Model
{
    public $obj_id;
    public $details;
}

$app = new Application($container);

$sql = <<<EOF
SELECT
    o.obj_name,
    o.obj_type,
    od.*
FROM
    Objects AS o
LEFT JOIN ObjectDetails AS od ON
    od.obj_id = o.obj_id
WHERE
    o.obj_id = :obj_id:
EOF;

/** @var \Phalcon\Mvc\Model\Query $query */
$query = $app->modelsManager->createQuery($sql);

print_r(explode(',', $query->getSql()['sql']));

Commit e851f48 introduces the following changes:

Array
(
+     [0] => SELECT "o"."obj_name" AS "o_obj_name"
-     [0] => SELECT "o"."obj_name" AS "obj_name"
+     [1] =>  "o"."obj_type" AS "o_obj_type"
-     [1] =>  "o"."obj_type" AS "obj_type"
      [2] =>  "od"."obj_id" AS "_od_obj_id"
      [3] =>  "od"."details" AS "_od_details"
      [4] =>  "od"."obj_type" AS "_od_obj_type" FROM "objects" AS "o"  LEFT JOIN "object_details" AS "od" ON "od"."obj_id" = "o"."obj_id" WHERE "o"."obj_id" = :obj_id
)

sergeyklay added a commit that referenced this issue May 7, 2020
@sergeyklay
Copy link
Contributor

Fixed in the 4.0.x branch. Feel free to open a new issue if the problem appears again. Thank you for the bug report, and for helping us make Phalcon better.

niden pushed a commit that referenced this issue May 16, 2020
niden pushed a commit that referenced this issue May 16, 2020
niden pushed a commit that referenced this issue May 31, 2020
niden pushed a commit that referenced this issue May 31, 2020
@niden niden added this to Phalcon v5 Aug 25, 2022
@niden niden moved this to Released in Phalcon v5 Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report status: low Low
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants