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

Database CHANGE from ENUM to TINYINT Fault #1989

Closed
bhsmither opened this issue Apr 29, 2018 · 3 comments
Closed

Database CHANGE from ENUM to TINYINT Fault #1989

bhsmither opened this issue Apr 29, 2018 · 3 comments
Assignees
Labels
Milestone

Comments

@bhsmither
Copy link
Contributor

In 6.2.0-b1.sql, a staement is issued to CHANGE CubeCart_geo_country.status to the TINYINT data type.

CubeCart has this column since 6.0.0 as an ENUM('0','1') text data type.

Be aware that the database actually stores the 1-based index of the list of strings:

[1] => "0"
[2] => "1"

Thus, when converting the contents of 'status' to TINYINT, it is the actual contents that remain - and not the result of attempting to convert the list element string literals that look like integers to actual integers.

Therefore, now that CC620's Status of Countries (CubeCart_geo_country.status ) use this scheme:

0: Disabled
1: Enabled (State Required)
2: Enabled (State Optional)
3: Enabled (State Disabled)

the result is that every (enabled) country in admin, Countries & Zones, Countries tab is now set to Enabled (State Optional).

Later, in 6.2.0-b1.sql, a statement is issued to set every country except a few to 'status'=2 -- which, as it turns out, accomplishes nothing.

Suggest:

ALTER TABLE `CubeCart_geo_country` CHANGE `status` `status` TINYINT(1)  NOT NULL  DEFAULT '1'; #EOQ
UPDATE `CubeCart_geo_country` SET `status` = `status`-1; #EOQ
UPDATE `CubeCart_geo_country` SET `status` = 2 WHERE `status` > 0 AND `iso` NOT IN('AR', 'BR', 'CA', 'CN', 'ID', 'IN', 'JP', 'MX', 'TH', 'US'); #EOQ
@abrookbanks
Copy link
Member

It should probably be ENUM('0','1','2') instead....

@bhsmither
Copy link
Contributor Author

bhsmither commented Apr 29, 2018

I have never seen commentary that promotes the use of ENUM to hold this sequence of numeric-looking strings. I have seen plenty of commentary that opines this is a bad idea.

ENUM has its uses, but a list of string literals used as zero- or one-based indexes to another list ain't one of them.

@abrookbanks abrookbanks self-assigned this May 2, 2018
@abrookbanks abrookbanks added the bug label May 2, 2018
@abrookbanks abrookbanks added this to the 6.2.1 milestone May 2, 2018
abrookbanks pushed a commit that referenced this issue May 2, 2018
What an oversight!
@abrookbanks
Copy link
Member

Tested with disabled countries on upgrade including US that needs state required. Passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants