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

Upgrader - 1.x issues #5752

Closed
wants to merge 10 commits into from
Closed

Conversation

sbulen
Copy link
Contributor

@sbulen sbulen commented Jul 26, 2019

Fixes #5741
Fixes #5750

**** I'm still testing this one... ****
**** In its current state, it addresses all issues in #5741 & #5750 except the utf8 problem. ****

**** Per discussion below, evaluating alternative approaches. ****

sbulen added 10 commits July 25, 2019 12:14
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Copy link
Collaborator

@albertlast albertlast left a comment

Choose a reason for hiding this comment

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

well 1004 think is a big change for below smf 2.1.
normaly you had to change the php code of all three version...

@@ -311,7 +311,10 @@ if (!empty($replaceRows))

---# Converting "log_activity"...
ALTER TABLE {$db_prefix}log_activity
ADD date date NOT NULL default '0001-01-01';
ADD date date NOT NULL default '1004-01-01';
Copy link
Collaborator

Choose a reason for hiding this comment

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

smf 1.0,1.1 and 2.0o can't handle 1004 in the way which 2.1 does,
the years 0000,0001 and in 2.1 1004 are somekind of special "year" which get handled differently.

Copy link
Contributor Author

@sbulen sbulen Jul 26, 2019

Choose a reason for hiding this comment

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

We are upgrading to the 2.1 code & logic...

While at the same time trying to navigate a bunch of silly rules... For example, mysql supports a DATE range of '1000-01-01' to '9999-12-31'.
https://dev.mysql.com/doc/refman/5.7/en/datetime.html

So... Where '0000-00-00' used to be an acceptable "null" value for a DATE, it is no longer with all these new STRICT rules of NO_ZERO_DATE and NO_ZERO_IN_DATE. Which, by the way, are going to be deprecated & folded into the definition of STRICT - so they will no longer be independent switches... They will be part of STRICT:
https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_no_zero_in_date

All the while, they hate '0000-00-00' so much, that you can't even use it in syntax to correct your OLD data (as we have seen in this SMF issue):
https://bugs.mysql.com/bug.php?id=85918

So... '0000-00-00' won't work, and '0001-01-01' is on the way out, we needed a "not specified" value. My understanding of the special SMF 2.1 treatment is that when it sees '1004-01-01' it doesn't display the date. That's why it's the new default.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well you can see this from two side,
the upgrader 1.0 and 1.1 should uprade to his related version number or
since we are here in 2.1 upgrade process we can leave the rule and targeting for 2.1 in every upgrade script.

The laste one would be the best way,
but i understand from other smf members that they don't like the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting dilemma, isn't it?

That's why I thought we should consider maybe operating different steps of the upgrader under different sql_modes. Only apply STRICT to the 2.1 step. If we do that, we could get by with many fewer updates. (This would allow us to leave all the 0000-00-00 logic in the 1.0 & 1.1 scripts.)

But that wouldn't fix all of the issues. To me, it appears that upgrade_query() has also changed behavior over the years. A rewrite of upgrade_query would result in many fewer changes needed. (The code appears to assume it will return false when tables/columns aren't found...)

It we want "untouched", universally-sharable 1.0 & 1.1 scripts, we would have to make the changes above to the upgrader.

Even then, there are still changes needed to the 1.0/1,1 scripts, due to confusion between the install & the code (e.g., mostOn vs most_on...).

Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm not sure how long the non sql mode hack works,
but i see the why this could be a valid way,

another way again would be to drop the 1.x upgrade path in 2.1,
So the admin had to use 2.0.x upgrade to move from 1.x to 2.0 and then use the upgrade of 2.1 to move again...

@sbulen
Copy link
Contributor Author

sbulen commented Jul 27, 2019

I am going to experiment with varying the sql_mode per step.

Who knows? It may even address the upgrade_query issues & the UTF8 issue.

@sbulen
Copy link
Contributor Author

sbulen commented Jul 29, 2019

I am going to close this one in preference to #5755

@sbulen sbulen closed this Jul 29, 2019
@sbulen sbulen deleted the fix_dem_zeroes branch August 7, 2019 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrader - challenges with 1.0 to 2.1 upgrade Upgrader - challenges with 1.1 to 2.1 upgrade
2 participants