Hi, I reported this on PortaMX forums.
They've suggested raising it here.
At the core of it, it looks like $smcFunc['db_insert']('replace', ... isn't working correctly. Instead of doing the replace, it's doing an insert and throwing Unique Key Constraint violations.
Anything I can do to fix this?
Cheers,
Mark
- - - - As reported to PortaMx
I'm getting a lot of errors on a pretty default install of portaMx.
My setup is SMF 2.0.5 and PortaMx 1.51-ecl.
For backend i'm using Postgresql 9.2.4 on FreeBSD.
I have a completely uncustomized SMF, just a couple of themes added, however I'm getting a lot of duplicate key errors trying to go to the various PortaMx admin tabs.
The main one is when i go to the portaMx tab:
ERROR: duplicate key value violates unique constraint "smf_portamx_settings_uidx"
DETAIL: Key (varname)=(liveinfo) already exists.
File: /usr/local/www/smf/Sources/PortaMx/AdminCenter.php
Line: 764
I found i can temporarily get around it by deleting the liveinfo row from the database, but after a couple of clicks around the admin pages, i can't go back without re-deleting the row.
I'm a java developer with some php experience, had a look at the source where the problem is, but i can't work out why the replace method of db_insert isn't actually doing either a delete/insert or an update but instead it's clearly trying to do an straight insert, and hence the duplicate key error.
I'm also seeing the same problem trying to set visibility of blocks on the site.
At first it just looked like the UI wasn't accepting my "update" or "update all" clicks in the blocks UI config, but if i get into the main configuration for a block (e.g. 'statistics' by clicking "click to edit this block") and try and update the user groups that are allowed to view the blocks, it comes back with a similar error on unique constraints, this time:
ERROR: duplicate key value violates unique constraint "smf_portamx_blocks_pkey"
DETAIL: Key (id)=(3) already exists.
File: /usr/local/www/smf/Sources/PortaMx/AdminBlocks.php
Line: 493
Are there any known issues with running on Postgresql 9.2, as I suspect there are d/b issues at work (similar to the main SMF install erroring because of the quoting change to \' vs '' in pgsql's change in 9+).
Off the top of my head a replace consists of two parts, a delete and insert so you would be right in saying that it looks to be doing an insert. If the replace isn't removing the record your looking to replace, you will of course get a duplicate key violation when it goes to insert the replacement record.
Is there any locks on the table your doing the replace on?
No, i can see no locks on the database:
blocked_pid | blocked_user | blocking_pid | blocking_user | blocked_statement
-------------+--------------+--------------+---------------+-------------------
(0 rows)
Probably because the query is wrong (https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/Sources/PortaMx/AdminCenter.php#L764): the last parameter used (http://wiki.simplemachines.org/smf/Db_insert) should not be the name of the variable inserted, but the name of the index, so in that case I think: 'uidx' (https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L71).
So, if you replace
array('liveinfo')
with:
array('uidx')
it should work.
On a side note:
https://github.com/SimpleMachines/SMF2.1/blob/release-2.1/Sources/Subs-Db-postgresql.php#L697
what the code is doing is exactly: delete and insert new.
Quote from: emanuele on October 01, 2013, 10:57:04 AM
Probably because the query is wrong (https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/Sources/PortaMx/AdminCenter.php#L764): the last parameter used (http://wiki.simplemachines.org/smf/Db_insert) should not be the name of the variable inserted, but the name of the index, so in that case I think: 'uidx' (https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L71).
So, if you replace
array('liveinfo')
with:
array('uidx')
it should work.
On a side note:
https://github.com/SimpleMachines/SMF2.1/blob/release-2.1/Sources/Subs-Db-postgresql.php#L697
what the code is doing is exactly: delete and insert new.
Thanks for the links, i've just been searching for what SMF is doing.
I'm not sure, however, your analysis is correct.
The replace is:
$smcFunc['db_insert']('replace', '{db_prefix}portamx_settings',
array(
'varname' => 'string',
'config' => 'string'
),
array(
'liveinfo',
serialize($info),
),
array('liveinfo')
);The function signature is:
function smf_db_insert($method = 'replace', $table, $columns, $data, $keys, $disable_trans = false, $connection = null)So the table is '{db_prefix}portamx_settings', columns are "varname, config", data is "liveinfo, <serialized liveinfo>", and the keys are "liveinfo".
The uidx is the constraint name for the primary key that's being violated, i.e. that there's already a row with liveinfo as the key.
I'm going to look at this deeper tonight. I put debug on my instance and tried to click the part that's breaking and only saw an INSERT happening, no delete. Also, the mod is pretty established, so I think that kind of error would have been picked up pretty quickly.
My current theory is that the delete isn't happening at lines 717-728 and i'll add debug to find out why later.
// A key? That's what we were looking for.
if (in_array($columnName, $keys))
So the name of the column, okay, not the name of the index, but for sure not the name of the variable you are inserting.
well spotted. changing to varname fixes the issue, i've posted back to my original bug report at http://portamx.com/3379/postgresql-duplicate-key-errors/
At least i should be able to fix the other issue now too.
Good, thanks for reporting back! ;D
We have a lot of insert - replace queries ...
So the solution is that the third array must hold the col name they have unique or primary index?
Correct ? ???
in this case these exist query must work:
$smcFunc['db_insert']('replace', '
{db_prefix}portamx_settings',
array(
'varname' => 'string',
'config' => 'string'
),
array(
'liveinfo',
serialize($info),
),
array('liveinfo')
);
Table structure:
'portamx_settings' => array(
// column defs
array(
array('name' => 'varname', 'type' => 'varchar', 'size' => '80', 'default' => '', 'null' => false),
array('name' => 'config', 'type' => 'text', 'null' => false),
),
// index defs
array(
array('type' => 'unique', 'name' => 'uidx', 'columns' => array('varname')),
),
// options
array()
),
Any remarks?
Looking at
http://wiki.simplemachines.org/smf/Db_insert
https://github.com/SimpleMachines/SMF2.1/blob/release-2.1/Sources/Subs-Db-postgresql.php#L697
the 3rd array are the primary key columns in the table, so that any columns you use in array 1 are matched for the delete's "WHERE" clause (lines 711/712 and 721 to 724).
Although I'll let the SMF guys confirm my reading of it.
It looks like you have it correct in https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/Sources/PortaMx/AdminCenter.php#L395 as an example.
The Insert "replace" routine for Postgres is definitive buggy (my meanings)...
I'v tested this and see that the delete row function can't work ..
The created delete query looks like this:
delete from tablename where id = {int:0}
The data for the insert and delete query looks like this:
$data[0] => [id]=26 => [more]=xx
so the smfFunc('delete') can't find the id in the data array.
The correct query for delete must look as follow:
delete from tablename where id = {int:id}
Now the row is correct deleted and the insert works.
So I think the "insert-replace" for postgres must be coded as follow:
// PostgreSQL doesn't support replace: we implement a MySQL-compatible behavior instead
if ($method == 'replace')
{
$count = 0;
$where = '';
foreach ($columns as $columnName => $type)
{
// Are we restricting the length?
if (strpos($type, 'string-') !== false)
$actualType = sprintf($columnName . ' = SUBSTRING({string:%1$s}, 1, ' . substr($type, 7) . '), ', $columnName);
else
$actualType = sprintf($columnName . ' = {%1$s:%2$s}, ', $type, $columnName);
// A key? That's what we were looking for.
if (in_array($columnName, $keys))
$where .= (empty($where) ? '' : ' AND ') . substr($actualType, 0, -2);
$count++;
}
// Make it so.
if (!empty($where) && !empty($data))
{
foreach ($data as $k => $entry)
{
$smcFunc['db_query']('', '
DELETE FROM ' . $table .
' WHERE ' . $where,
$entry, $connection
);
}
}
}
Any comment on this?
Quote from: feline on October 02, 2013, 05:29:38 AM
We have a lot of insert - replace queries ...
So the solution is that the third array must hold the col name they have unique or primary index?
Correct ? ???
And in your query the **column** name is liveinfo? (Or maybe varname?)
In the posted example the 3nd array have the colname ( 'id' ) while that is the indexed col...
So you're passing a 'key' of liveinfo into the insert function but don't apparently have a column or a key called that and wonder why it doesn't work? Just trying to make sense of this.
Quote from: feline on October 02, 2013, 05:29:38 AM
Table structure:
'portamx_settings' => array(
// column defs
array(
array('name' => 'varname', 'type' => 'varchar', 'size' => '80', 'default' => '', 'null' => false),
array('name' => 'config', 'type' => 'text', 'null' => false),
),
// index defs
array(
array('type' => 'unique', 'name' => 'uidx', 'columns' => array('varname')),
),
// options
array()
),
So if I'm not wrong the table should look like:
_________________________
| varname | config |
_________________________
| something1 | value |
| something2 | value |
| something3 | value |
| something4 | value |
| something5 | value |
| something6 | value |
| liveinfo | serial($) |
_________________________
Am I wrong?
No?
Then, the column name is liveinfo or varname?
If the column name is varname, then your query should be:
$smcFunc['db_insert']('replace', '{db_prefix}portamx_settings',
array(
'varname' => 'string',
'config' => 'string'
),
array(
'liveinfo',
serialize($info),
),
array('varname')
);
(of course the documentation is wrong).
Quote from: feline on October 02, 2013, 05:29:38 AM
We have a lot of insert - replace queries ...
There are 55 insert replace in SMF alone, I'm pretty sure that if the code was *so* buggy (in that specific point) it wouldn't even install on postgre. ;)
the problem isn't actually with the varname/liveinfo example, that's a string and works if you change to varname as i've seen on my site.
however, in feline's example, the id value (an int) doesn't work if you pass it in as "array('id')", there's a error saying "the value 0 is not in the database", so feline's example with the id column needs to also be looked at, but i haven't had time to further this tonight.
I'm still thinking that it's because it's being used wrong...
Does it occur anywhere with a base SMF install? Or is it only inside the PortaMX files?
Are you able to move topics from one board to another?
https://github.com/SimpleMachines/SMF2.1/blob/release-2.1/Sources/MoveTopic.php#L479
Are the boards marked as read properly?
Are you able to collapse and uncollapse categories?
Or even more simple: are you able to save *any* setting stored in the settings table?
https://github.com/SimpleMachines/SMF2.1/blob/release-2.1/Sources/Subs.php#L469
If so, the db-layer works.
i think i've bought an elephant to the party by installing with postgresql.
Partly, but SMF itself seems to work fine - sounds like the problem is PortaMX...
hopefully just a misreading of the API for pgsql, that we can work out and improve both sides by fixing.
Here's the thing... what SMF does works for SMF. If it didn't work, we'd fix it. But right now I haven't seen anything to indicate that SMF *itself* is misbehaving.
I tested that in a debugger ...
here the query we tested:
$smcFunc['db_insert']('replace', '
{db_prefix}portamx_blocks',
array(
'id' => 'int',
'side' => 'string',
'pos' => 'int',
'active' => 'int',
'cache' => 'int',
'blocktype' => 'string',
'acsgrp' => 'string',
'config' => 'string',
'content' => 'string',
),
$block,
array('id')
);
the indexed col is id.
I'v set a breakpoint just before the delete query starts.
In the attached images you can see the data in the variables.
Image1 is the original code (not work)
Image2 is the modificated code they I post (works)
and this code we use:
// PostgreSQL doesn't support replace: we implement a MySQL-compatible behavior instead
if ($method == 'replace')
{
$where = '';
foreach ($columns as $columnName => $type)
{
// Are we restricting the length?
if (strpos($type, 'string-') !== false)
$actualType = sprintf($columnName . ' = SUBSTRING({string:%1$s}, 1, ' . substr($type, 7) . '), ', $columnName);
else
$actualType = sprintf($columnName . ' = {%1$s:%2$s}, ', $type, $columnName);
// A key? That's what we were looking for.
if (in_array($columnName, $keys))
$where .= (empty($where) ? '' : ' AND ') . substr($actualType, 0, -2);
}
// Make it so.
if (!empty($where) && !empty($data))
{
$smcFunc['db_query']('', '
DELETE FROM ' . $table .
' WHERE ' . $where,
$data[0], $connection
);
}
}
Please try it self ...
Not technically an SMF bug: it is intended.
The array of data is not supposed to be an associative array, but an array with numerical indexes.
So, for example in AdminCategories.php, the $category array should be:
// get all data
$category = array(
$_POST['id'],
PortaMx_makeSafe($_POST['name']),
$_POST['parent'],
$_POST['level'],
$_POST['catorder'],
(!empty($_POST['acsgrp']) ? implode(',', $_POST['acsgrp']) : ''),
(!empty($_POST['artsort']) ? implode(',', $_POST['artsort']) : ''),
pmx_serialize($_POST['config']),
);
Still in PortaMx most of the 'replace' are broken (in Postgre) because they pass the wrong argument. ;)
Then I tried to install PortaMx (1.51 from github) but all the queries updating settings miss the index so they fail:
https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L333
https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L452
https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L593
https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L631
https://github.com/PortaMx/PortaMx-1.51-ecl/blob/master/install/dbinstall.php#L641
to fix, replace the last array with:
array('varname')
or
array('variable')
Though, out of curiosity: why are you adding hooks with an insert and not through add_integration_function and why are you updating the SMF settings table with an insert and not with updateSettings?
Well .. I found the problem ..
the SMF insert_replace data array MUST have a numeric index (see Image22).
Because we use a data array with a colname index (see Image11), the Postgres delete fails.
That a numeric index is a MUST HAVE is not documented on SMF ..
AND .. a correct programmed function can handle both (my meanings) .. but I'm not a SMF developer :laugh:
We just have updated the GIT repository for PortaMx 1.51 ecl (https://github.com/PortaMx/PortaMx-1.51-ecl)
Notice:
update all db insert-replace because SMF don't support string indexes in the data array (pew)
The install is still broken. :P
Thanks Feline, I'll update it later on. I trust this is just a code change so no d/b changes?
only changes on the PortaMx code...
Great work Feline, the site's working with all the updates, no more d/b errors.
Thanks!