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

Better 1.1 upgrade logic #5746

Closed

Conversation

albertlast
Copy link
Collaborator

dunno if i find more stuff,
for the first part i looked into the calendar, calendar_holidays and member table.

All this three table get touched again in 2.1 upgrade script:
https://github.com/SimpleMachines/SMF2.1/blob/e0848c82fbbc912b51789a2fefded6b6830f086f/other/upgrade_2-1_mysql.sql#L3-L36

So it make less sense todo this in 1.1 part before.
Secondly was here mayor mistake done,
the index eventData got dropped (this is for this case here very stupid).

issue: #5741

Signed-off-by: albertlast albertlast@hotmail.de
Signed-off-by: albertlast albertlast@hotmail.de
Signed-off-by: albertlast albertlast@hotmail.de
@sbulen
Copy link
Contributor

sbulen commented Jul 21, 2019

I don't think this will work - the 1.1 upgrade actually renamed some 1.0 columns and added others. I think those parts need to be kept. The 2.1 logic does not do the renames & new columns. (It goes from 'eventDate' to 'startDate' and 'endDate'.)

I think the only parts that need to be removed are the references to 0000-00-00.

Question: If there were invalid dates, I believe those would have been caught during the mysql upgrade itself, correct? I.e., it wouldn't support allowing those values in the columns at all? I.e., I think they would have been required to be corrected before even running this upgrader... If that is true, I believe it is safe to remove those checks for 0000-00-00.

@albertlast
Copy link
Collaborator Author

About the renaming i had to think about...

Invalid value get notice by mysql when you do a dml command.
So you can select a invalid date but you can't insert a invalid or update a field to unvalid one.
In this case here #5741 the issue is that the column get renamed by this invalid dates get "inserted".

So the order is wrong in the upgrade script -> first change the values and then rename the col.

Signed-off-by: albertlast albertlast@hotmail.de
@albertlast
Copy link
Collaborator Author

@sbulen is that so better?

@sbulen
Copy link
Contributor

sbulen commented Jul 23, 2019

No, same error. I believe you really need to get rid of all references to 0000-00-00...

(I'm running MySQL 5.7.17)

image

@jdarwood007
Copy link
Member

The upgrade scripts should not be touched unless they are currently broken to where they don't operate. The intention of those scripts is to upgrade you to that SMF versions compliant database. Upgrade 1-1 is meant to get you to a SMF 1.1 compliant database.

@albertlast
Copy link
Collaborator Author

nothing new to it,
since the world is changing the old env not exists any more,
so you had to upgrade the of the 1-1 upgrade to be able to run in our time.

@sbulen
Copy link
Contributor

sbulen commented Jul 24, 2019

Of course, you're both right. The question is what to do about it.

I'm running MySQL 5.7, and I cannot upgrade a vanilla 1.1.21 DB due to these syntax issues. It really seems to have a hard time with the 0000-00-00 dates. I believe others trying to upgrade from 1.1 may experience the same, depending on their version of MySQL.

@albertlast - I think we should leave the file as untouched as possible. I have confirmed that removing the 4 commands that reference 0000-00-00, and modifying the one that uses it in an AND, addresses this issue. I can run a full upgrade from 1.1 to 2.1 with that change.

There may be other approaches - e.g., by experimenting with sql_mode to introduce a more lax mode when processing the older files. I haven't had luck here so far, though.

But just removing the references to 0000-00-00 works.

@sbulen
Copy link
Contributor

sbulen commented Jul 26, 2019

...and upon further reflection, just removing the references to 0000-00-00 just won't work. Specifically, the various data fixes dont get applied.

Note there are some other issues with this file - I updated the issue accordingly.

@albertlast - I hope you don't mind I'm working on a PR to address both the 1.1 issues I've found thus far... I have some thoughts that might address Sleepy's concerns as well.

@albertlast albertlast closed this Jul 26, 2019
@sbulen sbulen mentioned this pull request Jul 26, 2019
@albertlast
Copy link
Collaborator Author

Since 1.x didn't support postgres,
im not full care about.

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.

None yet

3 participants