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

Syntax problem on MySQL strict mode #1210

Closed
majimboo opened this issue Apr 25, 2016 · 7 comments
Closed

Syntax problem on MySQL strict mode #1210

majimboo opened this issue Apr 25, 2016 · 7 comments
Labels
component:core A fault that lies within the main framework of rAthena mode:prerenewal A fault that exists within the pre-renewal mode mode:renewal A fault that exists within the renewal mode priority:medium A fault that makes rAthena have significant repercussions but does not render rAthena unusable type:bug Issue that is a bug within rAthena

Comments

@majimboo
Copy link

majimboo commented Apr 25, 2016

Error: Invalid date '0000-00-00'

Case: Newer version of MySQL/MariaDB no longer allows the use of a ZERO_DATE

Solution: Change all default date/datetime value to the minimum allowed value which is 1000-01-01 00:00:00 or lets not kid ourselves and just use NULL for ZERO_DATE.

Affected:

  • Default values on SQL scripts
  • login.c (for default value of _M/_F registration)
  • other files...

Thoughts?

@majimboo majimboo reopened this Apr 25, 2016
@Jeybla
Copy link
Contributor

Jeybla commented Apr 26, 2016

Case: Newer version of MySQL/MariaDB no longer allows the use of a ZERO_DATE

Do you have a source (changelog) for this information?

I just found this:

The NO_ZERO_DATE mode affects whether the server permits '0000-00-00' as a valid date. Its effect also depends on whether strict SQL mode is enabled.

Which wouldn't be enabled by default imo.

@majimboo
Copy link
Author

majimboo commented Apr 26, 2016

As of MySQL 5.7.4, NO_ZERO_DATE is deprecated. In MySQL 5.7.4 through 5.7.7, NO_ZERO_DATE does nothing when named explicitly. Instead, its effect is included in the effects of strict SQL mode. In MySQL 5.7.8 and later, NO_ZERO_DATE does have an effect when named explicitly and is not part of strict mode, as before MySQL 5.7.4. However, it should be used in conjunction with strict mode and is enabled by default. A warning occurs if NO_ZERO_DATE is enabled without also enabling strict mode or vice versa. For additional discussion, see SQL Mode Changes in MySQL 5.7.

Source: here and here of the mysql official dev docs

@Jeybla
Copy link
Contributor

Jeybla commented Apr 26, 2016

Tanks! Also interesting from "SQL Mode Changes in MySQL 5.7":

ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, and NO_ZERO_IN_DATE are included in the default sql_mode value, which as a result includes these modes: ONLY_FULL_GROUP_BY, STRICT_TRANS_TABLES, NO_ZERO_IN_DATE, NO_ZERO_DATE, ERROR_FOR_DIVISION_BY_ZERO, NO_AUTO_CREATE_USER, and NO_ENGINE_SUBSTITUTION.

So yes. I think you're right. We should use NULL instead of ZERO_DATE.

@majimboo
Copy link
Author

majimboo commented Apr 26, 2016

I'm quite confused why NULL wasn't used in the first place. Changing it at this point, could possibly break peoples servers if they try to update and rebuild, or even for people who rely on FluxCP (which should also be updated).

We could add an update sql script that updates all ZERO_DATE data to NULL and alter the column to default to NULL (quite scary trying to patch someones live db). Then we can fix all the problems found in the code related to this issue.

Although, changing it to 1000-01-01 rather than NULL shouldn't break as much but its just not right.

@Lemongrass3110 Lemongrass3110 changed the title Any plan to update for latest mysql Syntax problem on MySQL strict mode May 5, 2016
@secretdataz secretdataz added status:confirmed Issue that has been validated by a developer to affect rAthena component:core A fault that lies within the main framework of rAthena priority:medium A fault that makes rAthena have significant repercussions but does not render rAthena unusable server:login labels Jun 11, 2016
@RadianFord
Copy link
Contributor

RadianFord commented Jan 20, 2017

Is there another way to fix this? because I am having the same issue with the topic creator. :(

@Jeybla
Copy link
Contributor

Jeybla commented Jan 21, 2017

#1621 (comment)
... Another way for a quick workaround is to set all DATETIME fields to "1000-01-01 00:00:00".

This is a closed issue, since it is a duplicate. So you should continue posting in the open one.

Jeybla added a commit to Jeybla/rathena that referenced this issue Feb 3, 2017
@Jeybla
Copy link
Contributor

Jeybla commented Feb 3, 2017

So I'd like to solve this issue cause it's kinda annoying.

First things first so I changed the sql files:
Jeybla@973f35c

But the login instantly crashed by using this database.
So I needed to change login/account.c aswell:

-	Sql_GetData(sql_handle, 10, &data, NULL); safestrncpy(acc->lastlogin, data, sizeof(acc->lastlogin));
+	Sql_GetData(sql_handle, 10, &data, NULL); safestrncpy(acc->lastlogin, (data!=NULL)?data:"", sizeof(acc->lastlogin));
 	Sql_GetData(sql_handle, 11, &data, NULL); safestrncpy(acc->last_ip, data, sizeof(acc->last_ip));
-	Sql_GetData(sql_handle, 12, &data, NULL); safestrncpy(acc->birthdate, data, sizeof(acc->birthdate));
+	Sql_GetData(sql_handle, 12, &data, NULL); safestrncpy(acc->birthdate, (data!=NULL)?data:"", sizeof(acc->birthdate));

The login will survive after this change, but there are much more errors to deal with.
Example:

SQL_SUCCESS != SqlStmt_BindParam(stmt,  9, SQLDT_STRING,    (void*)&acc->lastlogin,       strlen(acc->lastlogin))
		||  SQL_SUCCESS != SqlStmt_BindParam(stmt, 10, SQLDT_STRING,    (void*)&acc->last_ip,         strlen(acc->last_ip))
		||  SQL_SUCCESS != SqlStmt_BindParam(stmt, 11, SQLDT_STRING,    (void*)&acc->birthdate,       strlen(acc->birthdate))

These lines are for updating the lastlogin and birthdate in the login table. It fails, since lastlogin and birthdate are zero strings now. We could use again a special cause like if date is empty then update NULL. But thats not nice, too...

Maybe a Dev could offer a better solution or we should just stick with '1000-01-01 00:00:00'.

@aleos89 aleos89 added mode:prerenewal A fault that exists within the pre-renewal mode mode:renewal A fault that exists within the renewal mode type:bug Issue that is a bug within rAthena and removed status:confirmed Issue that has been validated by a developer to affect rAthena labels Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:core A fault that lies within the main framework of rAthena mode:prerenewal A fault that exists within the pre-renewal mode mode:renewal A fault that exists within the renewal mode priority:medium A fault that makes rAthena have significant repercussions but does not render rAthena unusable type:bug Issue that is a bug within rAthena
Projects
None yet
Development

No branches or pull requests

5 participants