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

V156 zc install - add parser for left join queries #1981

Merged
merged 6 commits into from Dec 29, 2018

Conversation

mc12345678
Copy link
Contributor

@mc12345678 mc12345678 commented Dec 27, 2018

There have recently been multiple posts to the ZC forum about the upgrade process not completing successfully even though the screen message alludes to everything being done. While some of the issues appear to be related to date or datetime fields having '0000-00-00' stored in them (beyond the date_modified field already addressed) also identified a few items as captured in the comments for each commit of this pull request. One of the issues was that the sql statement to left join a table did not account for the DB_PREFIX.

EDIT: Recommend a method to evaluate the table data stored to see if the table's field definition is date or datetime and then if it could support a value of 0000-00-00 and if not then to modify the value either to null (if supported) or 0001-01-01, though also depends on what caused it and what can be done to handle the reset value. At the very least, suggest offering more/some information about the issue on the zc_install screen..

Done in preparation for future code.  No specific error was issued
to cause the code change.
Corrects issue of a DB_PREFIX on ZC 1.5.6 database upgrade
calling a 'LEFT JOIN ' query.
This appears to support logging of additional information in the
logs to more easily understand where in the process the operation
has reached.
{
if (!isset($result)) $result = sprintf(REASON_TABLE_NOT_FOUND, $this->lineSplit[2]).' CHECK PREFIXES!';
$this->writeUpgradeExceptions($this->line, $result, $this->fileName);
error_log($result . "\n" . $this->line . "\n---------------\n\n");
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to leave this error_log call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now able to respond in this location, I had intended the error_log to be here so that a zcInstallDEBUG-xxxxxxxxxx-xxxxxx file would be created in addition to populating a zcInstallLog_xx-xx-xxxx_xx-xx-xx-xxxxxx file though both are populated with information in this situation.

@drbyte
Copy link
Member

drbyte commented Dec 27, 2018

Thanks for these updates!

@mc12345678
Copy link
Contributor Author

Unable with my mobile to comment directly in line with the question of intentionally leaving the error_log statement.

In a way, yes, because otherwise it did not seem like a notification would be made in the debug logs that remain after the upgrade and then accessing the admin.

Even though I tried several different approaches, without a significant rewrite, it seemed like that was the best available option with minimal impact. That said though, the particular operation being performed it seems could cause the loss of ez_pages if someone were to attempt to do the upgrade on a site that doesn't have a backup of the database and this issue is encountered. :/

…set.

This is written to expect the sql statement to possibly be as minimal as
`DROP TABLE X` in which case `lineSplit[3]` would not have been set.  This
also removes the need to test if `lineSplit` is set and just use the
existing test conditions.
@drbyte drbyte changed the title V156 zc install partial correction V156 zc install - add parser for left join queries Dec 29, 2018
@drbyte drbyte merged commit 5af4a2f into zencart:v156 Dec 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants