we should take care of the update path, of course, but a good move

Is this something that may happen again switching to VARBINARY?
#9423

@joeforjoomla

I do not know yet.
If someone use join to session table using session_id column then it would be good to check that.

If someone use join to session table using session_id column then it would be good to check that.

There are people joining the session table on that column for reasons I do not comprehend (I didn't in 2015 and I don't in 2018, I have not found one legitimate use case where my database schema needs a FK to a table storing the session identifier).

A common use case to join the session table is for example when dealing with a chat extension allowing communication between guest users.

I did tests too, using MySQL 5.1, 5.5 and 5.7
In all cases with success, no errors are thrown because of the different field type/charset/collation so defintely there should not be any problem.

@joeforjoomla Please mark your test at Issue Tracker. Thanks.

It is too early to mark the test as a success.

This PR probably has to wait until #19687 will be merged.

@csthomas #19750 mentioned adding an index on guest column. Would that be another improvement to consider?

Yes, we can probably use the userid column instead of using guest. Column userid is already indexed.
If there is a relation like: if guest==0 then userid!=0 and if guest==1 then userid==0 then it should be ok.

I updated the code. Now this PR is ready for testing.

@joeforjoomla Could you re-test this PR.

@BrainforgeUK Because you know the subject well, could you test it?

@csthomas I re-tested this PR again up to MySQL 8.0 without any issue when joining between #__session.session_id AS varinary(192) and #__customtable.session_id AS varchar(191)

However i can't ensure that everything will work correctly on other RDBMS such as MariaDB, Postgresql, etc

@joeforjoomla thanks,

It is enough, great, now we need one more test and one of the maintainers will decide.

@joeforjoomla please mark your Test as successfully:

• open Issue Tracker
• Login with your github-Account
• Click on blue "Test this"-Button above Authors-Picture
• mark your Test as successfully
• hit "submit test result"

I have tested this item ✅ successfully on 104f824

I have tested this item ✅ successfully on 104f824

@ggppdk

The SQL parsing code for every ALTER TABLE statement seems to handle only 1 altering sub-clause case per ALTER TABLE statement

In short, joomla will see only the last sub-clause (to compare difference with db), but db did all clauses.

Explanation for PostgreSQL,

ALTER TABLE "#__session" ALTER COLUMN "time" DROP DEFAULT,
                         ALTER COLUMN "time" TYPE integer USING "time"::integer;
ALTER TABLE "#__session" ALTER COLUMN "time" SET DEFAULT 0;

** The first line ** removes the default value of the time column, but joomla did not see it because there is a second line (after the comma) - joomla should not know that the default value is removed, because it back in the third line,
in the second line, we change the column type, and now joomla sees it,
in the third line we set a new default column value and joomla also sees it.

We use two operations in one "ALTER TABLE" so that joomla can not see the change in the first line. This is useful when we can not directly change the default value (or change the INDEX of column), but first we need to remove the default value and then add another value.

I have tested it again a few minutes ago (by update) and in pgAdmin3 now I have:

  "time" integer NOT NULL DEFAULT 0,

Joomla on PostreSQL before click on "Database Fix"

I have tested this item ✅ successfully on 104f824

can't say i was able to meter performance gain, but it's ok for me

Ready to Commit after two successful tests.

Thank you all for your interest.

Merged to 3.9-dev via 6d1e90f