@mbabker @wilsonge outside of the google mailing list is there another way to get testers for this PR? Or can this be merged by code review?

@photodude testing now ?

I have tested this item ? unsuccessfully on 78798b2

I get

1062 Duplicate entry '19-0' for key 'PRIMARY' SQL=ALTER TABLE `#__modules_menu` MODIFY `menuid` int(11) UNSIGNED NOT NULL DEFAULT '0';

on

Database Version    5.5.28 
PHP Version     5.4.45

@wojsmol what version of Joomla? That looks like it's coming from an unrelated change.
Please also include all of the messages from the database fixer (like which SQL file it's not working with)

@photodude staging dowloaded after #10538 (comment)

Please try checking the staging branch without this patch and verify that the error doesn't show up.

A number of changes happened between 3.5.1 when I wrote this and the current staging. This patch should not affect the record adds which would be resulting in the duplicate key.

@photodude
2 cases:

• The lastest staging with testing sample data + #10098 + this PR - see #10538 (comment)
• The lastest staging without sample data + #10098 this PR - success

additional note

Database Collation  utf8_general_ci
Database Connection Collation   utf8mb4_general_ci 

@wojsmol in your testing did you get a message like `Table '_______' does not have column '_____' with type int(10). (From file 3.6.0-2016-06-01.sql.) ?

I need those messages to have a better idea of where the duplicate key is coming from and if it's related to this PR, the other PR or something else.

If I remember right Joomla 3.0+ has the option to choose which type of sample data to install:
So which Sample Data Set did you use?:

Blog English (GB) Sample Data
Brochure English (GB) Sample Data
Default English (GB) Sample Data
Learn Joomla English (GB) Sample Data
Test English (GB) Sample Data

@wojsmol I can not replicate the issue that caused your test to fail with either 3.5.1, 3.6.0.beta.2 or with the current staging. As far as I can tell it's likely something with your test environment. Can you try a completely fresh install from 3.6.0.beta.2 or current staging?

@photodude I retested now case with sampledata using following test procedure -
after each step checking whether the structure of the database is correct.

1. Install staging on commit 9e596d1 with Test English (GB) Sample Data

   Database table structure is up to date.

2. install patch tester v 2.0.1

   Database table structure is up to date.

3. Applay #10098 on commit f7c2ea3

   Table '#__contact_details' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__finder_links' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__finder_taxonomy' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__newsfeeds' does not have column 'id' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__newsfeeds' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   

4. Click fix button

   Table '#__contact_details' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__finder_links' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__finder_taxonomy' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__newsfeeds' does not have column 'id' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   Table '#__newsfeeds' does not have column 'access' with type int(10). (From file 3.6.0-2016-05-16.sql.)
   

5. Applay #10538 on commit 78798b2

   Table '#__banners' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__banners' does not have column 'cid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__banner_clients' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__categories' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__contact_details' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__contact_details' does not have column 'user_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__contact_details' does not have column 'catid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__content_frontpage' does not have column 'content_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__content_rating' does not have column 'content_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__contentitem_tag_map' does not have column 'content_item_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__extensions' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__menu' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__modules' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__modules_menu' does not have column 'moduleid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__modules_menu' does not have column 'menuid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__newsfeeds' does not have column 'catid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__overrider' does not have column 'id' with type int(10) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__postinstall_messages' does not have column 'extension_id' with type bigint(20) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__schemas' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__session' does not have column 'userid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__ucm_base' does not have column 'ucm_item_id' with type int(10) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__ucm_base' does not have column 'ucm_type_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__updates' does not have column 'update_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__updates' does not have column 'update_site_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__updates' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__update_sites' does not have column 'update_site_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__update_sites_extensions' does not have column 'update_site_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__update_sites_extensions' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__users' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__user_profiles' does not have column 'user_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   

6. Click fix button

   An error has occurred.
   1062 Duplicate entry '19-0' for key 'PRIMARY' SQL=ALTER TABLE #__modules_menu MODIFY menuid int(11) UNSIGNED NOT NULL DEFAULT '0';

7. Check database status one more time

   Table '#__modules_menu' does not have column 'menuid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__newsfeeds' does not have column 'catid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__overrider' does not have column 'id' with type int(10) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__postinstall_messages' does not have column 'extension_id' with type bigint(20) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__schemas' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__session' does not have column 'userid' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__ucm_base' does not have column 'ucm_item_id' with type int(10) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__ucm_base' does not have column 'ucm_type_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__updates' does not have column 'update_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__updates' does not have column 'update_site_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__updates' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__update_sites' does not have column 'update_site_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__update_sites_extensions' does not have column 'update_site_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__update_sites_extensions' does not have column 'extension_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__users' does not have column 'id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   Table '#__user_profiles' does not have column 'user_id' with type int(11) unsigned. (From file 3.6.0-2016-05-16.sql.)
   

Thanks @wojsmol for the additional information. Everything looks as I expect things to look, except for the Duplicate entry error. From the database status you can see what this patch is correcting. Maybe something in #10098 or Maybe #10098 needs to address something in the sample data.

Try applying this patch #10538 before #10098

@mbabker Do you have any thoughts on what might be causing the Duplicate entry error only when the sample data is included?

@wojsmol I'm almost certain the issue is with either the sample data or with #10098. I've added a test file with a smaller group that should allow testing of this PR without interference from that duplicate entry error.

3.6.0-2016-06-20.zip

@photodude When testing with sql file from #10538 (comment) the test is successful.

@brianteeman Please alter my test result.

Thanks @wojsmol. Would you report your failed test under #10098 so we can keep that conversation going over there?

I'll alter the test instructions here as well.

@wojsmol Testing #10538 with #10098 using sample data should work now.

@alex7r would you test this PR so we can get it to RTC?

@photodude, I'll test ASAP, 1-2 days.

@photodude I have tested this item ✅ successfully on 78798b2

Tested successfully with signed and unsigned int, tinyint, both with different length.

@wilsonge @brianteeman can this be marked RTC with the two successful tests. Would be nice to see this in 3.6.0 if possible or at least 3.6.1.

Rtc

Sorry didn't see the request to change a test resukt

Thanks everyone.

@photodude You are changing int() to bigint() in #10098 so should the fixInteger() not use bigint as well?

@roland-d This PR just fix existing checks and it's don't include any other check, for wrong SQL for example.

@alex7r Yes I can see that but what is the use of adding int when in another PR we change everything again to bigint. Just making sure we are not doing duplicate work.

@roland-d well it's not about changing something to something, it's about allowing another option. Use case:
SQL contains field with int(8) unsigned.
this will never be fixed by Joomla + passing SQL integrity checks, as check of type is generated wrong.

@alex7r I am merely trying to understand what is going on here, so I appreciate your input.

When I have a field like test INT(8) UNSIGNED this is wrong?

@roland-d I'm at PC now so can explain it fully I think, here:
There is nothing wrong having field like test INT(8) UNSIGNED,
But
Joomla's SchemaItem generates wrong check for it,
so Joomla thinks this field in your DB has wrong type and throws alert.
If
you'll try to fix this with Joomla, or manually trigger alter sql
your field still will have same type and Joomla still will complain about this,
because
check generated is wrong.
This PR
fixes generation of check sql, so Joomla treat it right.

Thank you for the explanation @alex7r

@alex7r Thanks for taking the time to fill in that additional explanation.

@roland-d it's basically as @alex7r said. I would point out from the PR description. "Basically, the fixInteger() function makes an overly broad assumption that any type passed in type1 either needs to be returned as that type or if it's the full word type integer that it should check if the type2 is unsigned."

So prior to this change fixInteger() was just enforcing the full word type integer to be int(10) and unsigned if designated unsigned; this is partly due to how mysql versions treat the word full word type integer. This fix broadened that function, as @alex7r pointed out, to ensure that Joomla's SchemaItem didn't ignore cases where fixInteger()'s parameter type2 is unsigned.

Glad to see this is merged. Hopefully we can now work through getting some better consistency in the database scheme as noted in #10077 and partially addressed in #10098

@photodude Thank you as well for explaining this further. It is not that I have anything against this PR but as mentioned before I was just trying to understand what it was doing. Now it is all clear to me 👍