search

Buzzinoffbond / phpliteadmin

Automatically exported from code.google.com/p/phpliteadmin
0 stars 0 forks source link

SQL tab should always expect rows #227

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Currently, phpliteadmin uses

    if(preg_match('/^\s*(?:select|pragma)\s/i', $query[$i])===1)

to check if it should expect rows.

But I think that it's a little buggy, because there could be other keywords to 
select rows (e.g `explain`, see issue 224), or other keywords could appear in 
the future.

Then, I propose using

    $queryTimer = new MicroTimer();
    $result = $db->selectArray($query[$i], "assoc");
    $queryTimer->stop();

    echo "<div class='confirm'>";
    echo "<b>";
    // 22 August 2011: gkf fixed bugs 46, 51 and 52.

    $error = $db->getError();
    if($error === NULL)
    {

        printf($lang['show_rows'], sizeof($result));
        echo $db->getAffectedRows()." ".$lang['rows_aff']." ";

        printf($lang['query_time'], $queryTimer);
        echo "</b><br/>";
    }
    else
    {
        echo $lang['err'].": ".$error."</b><br/>";
    }

instead of

    $queryTimer = new MicroTimer();
    if(preg_match('/^\s*(?:select|pragma)\s/i', $query[$i])===1)   // pragma often returns rows just like select
    {
        $isSelect = true;
        $result = $db->selectArray($query[$i], "assoc");
    }
    else
    {
        $isSelect = false;
        $result = $db->query($query[$i]);
    }
    $queryTimer->stop();

    echo "<div class='confirm'>";
    echo "<b>";
    // 22 August 2011: gkf fixed bugs 46, 51 and 52.
    if($result)
    {
        if($isSelect)
        {
            $affected = sizeof($result);
            printf($lang['show_rows'], $affected);
        }
        else
        {
            $affected = $db->getAffectedRows();
            echo $affected." ".$lang['rows_aff']." ";
        }
        printf($lang['query_time'], $queryTimer);
        echo "</b><br/>";
    }
    else
    {
        echo $lang['err'].": ".$db->getError()."</b><br/>";
    }

in both "database SQL editor" and "perform SQL query on table".

I am using
* phpLiteAdmin v1.9.4.1 
* PHP 5.4.17
* PDO
* SQLite version 3.7.7.1
* Apache 2.4.6
* Windows XP SP3

Original issue reported on code.google.com by Loiroor...@gmail.com on 30 Aug 2013 at 5:28

GoogleCodeExporter commented 9 years ago 
Maybe it can be improved a bit, so that:
* If there aren't selected rows but there are modified ones, phpliteadmin will 
only say the number of modified ones.
* If there are selected rows but there aren't modified ones, phpliteadmin will 
only say the number of selected ones.
* Otherwise, phpliteadmin will say both numbers.

Something like

    if(sizeof($result) || !$db->getAffectedRows())
    {
        printf($lang['show_rows'], sizeof($result));
    }
    if($db->getAffectedRows() || !sizeof($result))
    {
        echo $db->getAffectedRows()." ".$lang['rows_aff']." ";
    }

instead of just

    printf($lang['show_rows'], sizeof($result));
    echo $db->getAffectedRows()." ".$lang['rows_aff']." ";

Original comment by Loiroor...@gmail.com on 3 Sep 2013 at 2:52

GoogleCodeExporter commented 9 years ago 
Agreed.

Original comment by crazy4ch...@gmail.com on 28 Dec 2014 at 12:38

GoogleCodeExporter commented 9 years ago

Original comment by crazy4ch...@gmail.com on 28 Dec 2014 at 12:39

GoogleCodeExporter commented 9 years ago 
There is a problem with this approach of getting the error. Executing this 
query in the SQL tab:
SELECT non_existing FROM "test";
SELECT * FROM "test";

The first query will show "ERROR: no such column: non_existing" correctly. But 
the second one will also show this, because getting the last error still 
returns this error.
Instead of checking if getError() is NULL, we need to check if $result is not 
NULL.

Other than that the proposed fix is good. I am going to commit it in a minute.

Original comment by crazy4ch...@gmail.com on 28 Dec 2014 at 2:06

GoogleCodeExporter commented 9 years ago 
Fixed with revision 1f93ebad304820b2418ed2231a4827a89bb0776a.

Original comment by crazy4ch...@gmail.com on 28 Dec 2014 at 2:14