1.5.7 - "Query incomplete: missing closing semicolon."

Hey all! I'm sure it's something on my end, but wanted to report it in case it wasn't.

I uploaded a new install of 1.5.7, and I am porting over a bunch of plugins. Whenever I use the Admin --> SQL Query Executor and paste in a MySQL statement, we always get the following error: "Query incomplete: missing closing semicolon."

However, it's always followed by the successful statement, and the entries do seem to make it into the database.

Screenshot: https://i.postimg.cc/9fjQJ72Y/screenshot-1242.png

I checked the logs directory and it's not flagging any log errors. Just curious as to why this may be happening.

So have you seen the little "comment" just above the sql box that reads: "Be sure to
end with ;"

This is saying to include the semi-colon at the end of each statement. Now the screen shot provided does not show the sql statement(s) that were presented to the processor. So it is difficult to determine what the breakdown is or whether there really was a problem. My guess though is that there were some 13 different queries executed and none of them had a semi-colon (;) at the end of the line...

I also suspect that the processor is smart enough to figure out the end of the line and process each query.

mc12345678:

So have you seen the little "comment" just above the sql box that reads: "Be sure to
end with ;"

This is saying to include the semi-colon at the end of each statement. Now the screen shot provided does not show the sql statement(s) that were presented to the processor. So it is difficult to determine what the breakdown is or whether there really was a problem. My guess though is that there were some 13 different queries executed and none of them had a semi-colon (;) at the end of the line...

I also suspect that the processor is smart enough to figure out the end of the line and process each query.

It's definitely NOT that. Each line is definitely terminated by a semicolon. That was the first thing I checked. And the queries DO execute successfully and insert themselves into the database, despite those error messages. That is why I was confused.

Think I might have found a cause for the missing semicolon.
if you have a statement that is just a comment and does not end with a semi colon you get the query incomplete missing semi colon.
so for example:```
/* Surface mail Europe */
UPDATE configuration SET configuration_value = '0.1:5, 0.25:5.2, 0.5:7.1, 0.75:8.35, 0.1:9.55, 0.125:10.45, 1.5:11.5, 1.75:12.3, 2:12.75' WHERE configuration_key = 'MODULE_SHIPPING_RMSMPARCEL_ZONES_COST0_1' ;

/* Surface mail Rest of World */
UPDATE configuration SET configuration_value = '0.1:5, 0.25:5.2, 0.5:7.1, 0.75:8.35, 0.1:9.55, 0.125:10.45, 1.5:11.5, 1.75:12.3, 2:12.75' WHERE configuration_key = 'MODULE_SHIPPING_RMSMPARCEL_ZONES_COST0_2' ;

will generate

if you add semi colons to the end of the comments you get 4 statements successfully processed.

Similarly if you add lines to make a query more read able you also get the message (even if you edit the query on the server in my case Linux ubuntu).
so 

/* Surface mail Europe */;
UPDATE configuration
SET configuration_value = '0.1:5, 0.25:5.2, 0.5:7.1, 0.75:8.35, 0.1:9.55, 0.125:10.45, 1.5:11.5, 1.75:12.3, 2:12.75'
WHERE configuration_key = 'MODULE_SHIPPING_RMSMPARCEL_ZONES_COST0_1' ;

/* Surface mail Rest of World */;
UPDATE configuration SET configuration_value = '0.1:5, 0.25:5.2, 0.5:7.1, 0.75:8.35, 0.1:9.55, 0.125:10.45, 1.5:11.5, 1.75:12.3, 2:12.75' WHERE configuration_key = 'MODULE_SHIPPING_RMSMPARCEL_ZONES_COST0_2' ;

will generate the same two missing semicolon messages and say that 4 statements have been successfully processed.

Finally as you can see a blank line with nothing on it is ignored.

Not sure if this is new intended behaviour but definitely new in 1.5.7.

I am using 1.5.7-06232020 PHP 7.4

(Hard lesson learned several years ago) Commenting with /* and */, //, or any such "PHP" style comments is not appropriate in sql... SQL comments should begin with a hashtag (#) generally followed by a space...

As such, if those "php" type comments were prefixed with # then the response would be to have processed 2 lines instead of 4 considering the above demonstration sql..

This is also supported in the admin/sqlpatch.php code at line 91:

if (substr($line, 0, 1) != '#' && substr($line, 0, 1) != '-' && $line != '') {

This actually only checks to see if the first character is # not necessarily that it has a space after it and the result is for the line to be bypassed during processing.

Also found that "--" works. so ```
-- Surface mail Europe
UPDATE configuration SET configuration_value = '0.1:5, 0.25:5.2, 0.5:7.1, 0.75:8.35, 0.1:9.55, 0.125:10.45, 1.5:11.5, 1.75:12.3, 2:12.75' WHERE configuration_key = 'MODULE_SHIPPING_RMSMPARCEL_ZONES_COST0_1' ;

-- Surface mail Rest of World
UPDATE configuration SET configuration_value = '0.1:5, 0.25:5.2, 0.5:7.1, 0.75:8.35, 0.1:9.55, 0.125:10.45, 1.5:11.5, 1.75:12.3, 2:12.75' WHERE configuration_key = 'MODULE_SHIPPING_RMSMPARCEL_ZONES_COST0_2' ;

Works fine!

Yes agree hard lesson learnt!

Still does not explain why multi-line queries generate the error in 1.5.7 but worked fine in 1.5.6

New Problem!
the following will only work if it is over 2 lines which leads to a missing semi colon message.

# 
# Add the attribute match parameter for all existing installed modules
#

INSERT IGNORE INTO configuration (configuration_title, configuration_key, configuration_value, configuration_description, configuration_group_id, sort_order, set_function, date_added) 
SELECT 'Attribute Exact Match', CONCAT( LEFT( configuration_key , LOCATE( '_STATUS', configuration_key)), 'ATTRIBUTE_MATCH' ), 'False', 'Used to only display this shipping method if the attribute shipping is and exact match', '6', '0', "zen_cfg_select_option(array('True', 'False'), ", now() FROM configuration WHERE configuration_key LIKE 'MODULE_SHIPPING_RM%_STATUS';

If it is on a single line the sql is translated as

INSERT IGNORE INTO zen_configuration (configuration_title, configuration_key, configuration_value, configuration_description, configuration_group_id, sort_order, set_function, date_added) SELECT 'Attribute Exact Match', CONCAT( LEFT( configuration_key , LOCATE( '_STATUS', configuration_key)), 'ATTRIBUTE_MATCH' ), 'False', 'Used to only display this shipping method if the attribute shipping is and exact match', '6', '0', "zen_cfg_select_option(array('True', 'False'), ", now() FROM configuration WHERE configuration_key LIKE 'MODULE_SHIPPING_RM%_STATUS';

The second configuration is not prefixed with the zen_

is there a way of saying the query continues on to a second line to allow zencart to process the query in multiple lines with out the Missing semicolon error.

Have tried #NEXT_X_ROWS_AS_ONE_COMMAND:2
if i put on a ; at the end if the first line (INSERT) if fails because the insert is not complete. if i leave it off it works but reports missing semicolon.

Not sure how to group the last couple of posts, but they seem to all relate... I would say that this is a bug...

Here's why...

For one, admin/sqlpatch.php is not exactly a full sql parser... By that I mean, there are some characteristics and things that it does or tries to do to make it easier to implement than to go to phpMyAdmin and try to execute the sql statement(s). So it has some "rules" or formats to consider... That said, in the "long" sql above, the parser recognizes the initial instruction of 'INSERT IGNORE INTO ...' and can identify where the table is to be identified and then take the action necessary to include it; however, a subsequent sql "statement" that uses a table within that line is not recognized... Hence the need for "multiple lines".

So, then the logic in the multiple line processing doesn't appear quite right as:

      if (substr($line, -1) == ';') {
        //found a semicolon, so treat it as a full command, incrementing counter of rows to process at once
        if (substr($newline, -1) == ' ') {
          $newline = substr($newline, 0, (strlen($newline) - 1));
        }
        $lines_to_keep_together_counter++;
        if ($lines_to_keep_together_counter == $keep_together) { // if all grouped rows have been loaded, go to execute.
          $complete_line = true;
          $lines_to_keep_together_counter = 0;
        } else {
          $complete_line = false;
        }
      } //endif found ';'
        else {
            $messageStack->add(ERROR_LINE_INCOMPLETE, 'error');
        }

The first group (where the completion of the line is evaluated) is not entered unless the line ends with a semi-colon, but as pointed out if it ends with a semi-colon then ultimately when all lines are attempted to be processed as one mysql will complain that there is an ending statement where one should not be (my words not mysql's). But if the semi-colon is omitted then in a way, the command doesn't appear that it will complete as '$complete_line' won't reach a true status until effectively the appropriate number of semi-colons have been reached to account for the initial grouping...

What I have had to do in situations like you have described (as a work around rather than a fix) is to personally add the DB_PREFIX at the later locations in the query that is on the single line, or "ignore" the missing semi-colon statement if execution is determined to be satisfactory...

In some cases the #NEXT_X_ROWS_AS_ONE_COMMAND:2 concept is to ensure that two (the number of identified commands to keep together) different actions are performed as a single transaction (assume you retrieve the total number of sales or some other "dynamic" data that are then used to perform a subsequent action. By them processing together, the expectation is that no external action will modify the state of that information between the first and second operation). This is different than a single transaction that carries over multiple lines (the two line query presented above)...

I can say that the zc_install process uses in some cases the #NEXT_X_ROWS_AS_ONE_COMMAND:X concept for a multiline query and other cases it doesn't. At my last touch of some of those operations I suggested use based on the information that the system provided back and its importance to say trouble shooting or collecting information. Possibly a backwards way to have approached the identified issues, but anything/everything can be made better. :) Eventually...

What might "make it better" would be in the else section, to possibly also evaluate the condition of the line and evaluate if there are lines remaining in the keep_together count... If there are no more lines to process, then present the message. If there are still lines to process then carry on and increment the counter... But... That also assumes that the '#NEXT_X_ROWS_AS_ONE_COMMAND:X' flag was/is also intended to represent processing of a single query that is presented over many lines... (I think so, but I'm just another forum member.)

This could possibly become something like:

      if (substr($line, -1) == ';') {
        //found a semicolon, so treat it as a full command, incrementing counter of rows to process at once
        if (substr($newline, -1) == ' ') {
          $newline = substr($newline, 0, (strlen($newline) - 1));
        }
        $lines_to_keep_together_counter++;
        if ($lines_to_keep_together_counter == $keep_together) { // if all grouped rows have been loaded, go to execute.
          $complete_line = true;
          $lines_to_keep_together_counter = 0;
        } else {
          $complete_line = false;
        }
      } //endif found ';'
        else {
        if (substr($newline, -1) == ' ') {
          $newline = substr($newline, 0, (strlen($newline) - 1));
        }
        $lines_to_keep_together_counter++;
        if ($lines_to_keep_together_counter == $keep_together) { // if all grouped rows have been loaded, don't execute because don't have an ending semi-colon.
            $messageStack->add(ERROR_LINE_INCOMPLETE, 'error');
            $complete_line = false; // This may be unnecessary in the grand scheme of things, but it prevents the line from being processed.
            $new_line = '';
        }

        }

Other thing to note though, is I believe the '#NEXT_X_ROWS_AS_ONE_COMMAND:X' feature is not properly implemented... Looking at the loop code as pseudocode:
go through a line of the sql.
Set $keep_together = 1;
Evaluate the current line to see if it suggests additional lines to keep together.
Evaluate the line for a semi-colon, if present and the number of semi-colons ($lines_to_keep_together_counter == $keep_together) have been reached then add up the content together and process. If the semi-colon is present and there are more lines to be processed then move on to the next line (this effectively returns to the top, but lets carry on) but increment $lines_to_keep_together_counter.

get the new line of the sql
set $keep_together = 1;
Evaluate this line to see if it suggests additional lines to keep together. (on the second line this will be effectively false which means that keep_together remains a value of 1).
if the current line contains a semi-colon, then increment the counter ($lines_to_keep_together_counter) but now, $lines_to_keep_together_counter will exceed $keep_together because of the reassignment to 1 instead of keeping it to the value that was previously found/identified... Which means that the query will still not execute and there seems to run the possibility that this group of query won't actually execute at all...
A possible solution to this last part might be:
at the completion of the query, to have:

#NEXT_X_ROWS_AS_ONE_COMMAND:X;

Noting how a semi-colon is used at the end, but... Problem still with that is that the value X will not be a number, but will "eventually" be cast to be of the same data type as the counter unless php and associated system settings (strict control for example) offer an myDebug message about attempting to evaluate (2==2;)

Btw, the message associated with the multi-line query (no '#NEXT_X_ROWS_AS_ONE_COMMAND:X' being used) is a result of:
https://github.com/zencart/zencart/pull/3288

In short, right now, would say that the missing semi-colon message is acceptable and will support processing, to use an existing "command of #NEXT_X_ROWS_AS_ONE_COMMAND:X" there appears to be a little work to be done on overall syntax/data tracking and/or a modification to the message being displayed such that it is not always displayed just because there exists a line in the sql that doesn't end with a semi-colon...

Thoroughly confused yet? I am highly certain that the one that prepared the above commit will be...

brittainmark:

Have tried #NEXT_X_ROWS_AS_ONE_COMMAND:2
if i put on a ; at the end if the first line (INSERT) if fails because the insert is not complete. if i leave it off it works but reports missing semicolon.

When you say that "it works" have you validated that the anticipated changes have been implemented, or just that there is no "missing semicolon" message?

In my brief review of the code, I expect the latter that it didn't actually execute.

Yes think it is a bug. Think I have a fix also.
First issue is $keep_together = 1; on line 81 is set each time the loop is executed. This needs to be moved outside the loop

  foreach ($lines as $line) {
    if ($_GET['debug'] == 'ON') {
      echo $line . '<br>';
    }

    $line = trim($line);
    $line = str_replace('`', '', $line); //remove backquotes
    $line = $saveline . $line;
    $keep_together = 1; // count of number of lines to treat as a single command

Move to line 72

   $keep_together = 1; // count of number of lines to treat as a single command
  foreach ($lines as $line) {
    if ($_GET['debug'] == 'ON') {
      echo $line . '<br>';
    }

    $line = trim($line);
    $line = str_replace('`', '', $line); //remove backquotes
    $line = $saveline . $line;
    
  

Second issue is you need to remove the semicolon from the end of the string before looping around to get the next line. around line 277 inserted lines in red. then works fine.

 if ($lines_to_keep_together_counter == $keep_together) { // if all grouped rows have been loaded, go to execute.
          $complete_line = true;
          $lines_to_keep_together_counter = 0;
        } else {
            if (substr($newline, -1) == ';') {
                $newline = substr($newline, 0, (strlen($newline) - 1)). ' ';
            }
          $complete_line = false;
        }

This may be a bit counter intuitive as the sql you are trying to execute is

INSERT IGNORE INTO configuration (configuration_title, configuration_key, configuration_value, configuration_description, configuration_group_id, sort_order, set_function, date_added) 
SELECT 'Attribute Exact Match', CONCAT( LEFT( configuration_key , LOCATE( '_STATUS', configuration_key)), 'ATTRIBUTE_MATCH' ), 'False', 'Used to only display this shipping method if the attribute shipping is and exact match', '6', '0', "zen_cfg_select_option(array('True', 'False'), ", now() FROM configuration WHERE configuration_key LIKE 'MODULE_SHIPPING_RM%_STATUS';

to get it to process correctly you need to enter

#NEXT_X_ROWS_AS_ONE_COMMAND:2
INSERT IGNORE INTO configuration (configuration_title, configuration_key, configuration_value, configuration_description, configuration_group_id, sort_order, set_function, date_added) ;
SELECT 'Attribute Exact Match', CONCAT( LEFT( configuration_key , LOCATE( '_STATUS', configuration_key)), 'ATTRIBUTE_MATCH' ), 'False', 'Used to only display this shipping method if the attribute shipping is and exact match', '6', '0', "zen_cfg_select_option(array('True', 'False'), ", now() FROM configuration WHERE configuration_key LIKE 'MODULE_SHIPPING_RM%_STATUS';

The lines are not counted unless they have the ; on the end.

May need a little explanation in the developers docs.

I am happy to make the changes and push them up.

Also would be happy to document how it works if necessary.

Thanks for your input.

I have tried putting in a multiline command of multiple sql command and it fails if i make the loop work correctly.

#NEXT_X_ROWS_AS_ONE_COMMAND:2 
select 1=1; 
select 1=2;

Debug output

#NEXT_X_ROWS_AS_ONE_COMMAND:2
select 1=1;
select 1=2;

About to execute.
Debug info:
$ line=select 1=2;
$ complete_line=1
$ keep_together=2
SQL=select 1=1;select 1=2;

WARNING: An Error occurred, please refresh the page and try again.If you were entering information, press the BACK button in your browser and re-check the information you had entered to be sure you left no blank fields.

As you can see it tries to execute both statements in a single SQL command.

Makes me think that it was intended to be for multi line statements.

just found this in the sqlpatch language file you can see it by clicking of details in the SQLpatch window

**Advanced Methods
**The following methods can be used to issue more complex statements.
To run some blocks of code together so that they are treated as one command by MySQL, you need the

#NEXT_X_ROWS_AS_ONE_COMMAND:xxx

> If you are running this file via phpMyAdmin or an equivalent, the "#NEXT..." comment is ignored, and the script will process fine.
> **NOTE:** SELECT.... FROM... and LEFT JOIN statements need the "FROM" or "LEFT JOIN" to be on a line by itself in order for the parse script to add the table prefix.
> 
> 
> ***Examples:***
> 
> #NEXT_X_ROWS_AS_ONE_COMMAND:4
> SET @t1=0;
> SELECT (@t1:=configuration_value) as t1 
> FROM configuration 
> WHERE configuration_key = \'KEY_NAME_HERE\';
> UPDATE product_type_layout SET configuration_value = @t1 WHERE configuration_key = \'KEY_NAME_TO_CHECK_HERE\';
> DELETE FROM configuration WHERE configuration_key = \'KEY_NAME_HERE\';


This suggests that lines do not end in a semi colon are valid!

Really don't know where to go next.

I already have scripts that reply on values set before and they work without the #NEXT_X_ROWS_AS_ONE_COMMAND

Just done a diff on 1.5.6 and 1.5.7 and found the following lines added about line 283
```php
 else {
            $messageStack->add(ERROR_LINE_INCOMPLETE, 'error');
        }

This is the reason for the incomplete line message. probably need to be moved so that if there is no ';' on the last statement then the error is produced.

Thoughts / next steps?