Skip to content

Conversation

@lulco
Copy link
Contributor

@lulco lulco commented Oct 15, 2016

No description provided.

@lulco
Copy link
Contributor Author

lulco commented Oct 15, 2016

In PHP 7.0.11 Postgres PDO lastInsertId() was changed: http://php.net/ChangeLog-7.php#7.0.11
It returns "0" instead of false

#hacktoberfest

: NULL
);
if ($primaryKey === FALSE) {
if ($primaryKey === FALSE || $primaryKey === "0") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the string should be in single quotes, i.e. $primaryKey === '0'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, fixed

@dg
Copy link
Member

dg commented Oct 17, 2016

What about simple if (!$primaryKey)?

@JanTvrdik
Copy link
Contributor

JanTvrdik commented Oct 17, 2016

@dg That's less obvious. I think the current solution is better.


Even better (again for clarity) might be to add comment or split the condition based on PHP_VERSION_ID or both, e.g.

if ($primaryKey === '0' && (PHP_VERSION_ID < 70011 && $primaryKey === FALSE)) { // php issue #72633

@lulco
Copy link
Contributor Author

lulco commented Oct 17, 2016

So? Which alternative will be accepted and merged? I am OK with all of them, just need this fix.

@dg
Copy link
Member

dg commented Oct 17, 2016

I dont like fix for specific DB in common class.

@lulco
Copy link
Contributor Author

lulco commented Oct 18, 2016

Well, I just realize that it is not just pgsql problem. $this->context->getInsertId() returns '0' also with MySql DB if you have non-autoincrement primary key. And in MySql DB it is happening also in previous PHP versions.

This part of code is executed:

$row = $this->createSelectionInstance()
            ->select('*')
            ->wherePrimary($primaryKey)
            ->fetch();

And returns FALSE, because the executed query is: ... WHERE primary_key = '0' ...

In this query is the only driver-specific problem with pgsql uuid type, because it throws exception:
Nette\Database\DriverException: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for uuid: "0". The exception in query that shouldn't be executed at all. Definitelly not with $primaryKey = '0'.

@dg dg force-pushed the master branch 12 times, most recently from 06e5f54 to 44a230d Compare October 18, 2016 12:57
@dg
Copy link
Member

dg commented Oct 18, 2016

It seems that it is '' in mssql

@hrach
Copy link
Contributor

hrach commented Oct 18, 2016

This seems to be a wrong fix; personally, I have a db with table with primay value 0 (yes, big bad design, but that's the current state) . The getLastIndsertedId() should be fixed in the proper layer (Driver/Connection), shouldn't be?

@dg
Copy link
Member

dg commented Oct 18, 2016

So '0' can be missing primary and correct primary? I don't know how to differentiate between them. The only solution I can see is to not support primary value '0'.

@hrach
Copy link
Contributor

hrach commented Oct 18, 2016

You can definitely insert '0' into a PK column in MySQL. I'm ok with not-supporting the PK '0', but the unification may be done in PgSQLDriver.

@dg
Copy link
Member

dg commented Oct 18, 2016

MySQL returns '0' when primary key is missing. It seems that condition in Selection was wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants