Navigation Menu

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

sqltable_to_php.php does not always generate valid table data arrays #2702

Closed
ddb4github opened this issue May 23, 2019 · 3 comments
Closed
Labels
bug Undesired behaviour resolved A fixed issue

Comments

@ddb4github
Copy link
Contributor

Describe the bug
db_update_table function does not support table data array that is generated by sqltable_to_php.php with args '--plugin' and '--update'

  • In lib/database.php: function db_update_table(...):
    $data['primary'] is always considered as array:
    if (isset($data['primary'])) {
        if (!isset($allindexes['PRIMARY'])) {
            // No current primary key, so add it
            if (!db_execute("ALTER TABLE `$table` ADD PRIMARY KEY(" . db_format_index_create($data['primary']) . ')', $log, $db_conn)) {
                return false;
            }
        } else {
            $add = array_diff($data['primary'], $allindexes['PRIMARY']);
            $del = array_diff($allindexes['PRIMARY'], $data['primary']);
            if (!empty($add) || !empty($del)) {
                if (!db_execute("ALTER TABLE `$table` DROP PRIMARY KEY", $log, $db_conn) ||
                    !db_execute("ALTER TABLE `$table` ADD PRIMARY KEY(" . db_format_index_create($data['primary']) . ')', $log, $db_conn)) {
                    return false;
                }
            }
        }
    }
  • sqltable_to_php.php output $data['primary'] depend arg '--plugin'
            if (!empty($pri)) {
                if ($plugin != '') {
                    $text .= "\$data['primary'] = '" . implode("`,`", $pri) . "';\n";
                } else {
                    $text .= "\$data['primary'] = array('" . implode("','", $pri) . "');\n";
                }
            }

Expected behavior
sqltable_to_php.php always output primary key as below if '--update' is specified:
$data['primary'] = 'host,graph';

cigamit added a commit that referenced this issue May 24, 2019
sqltable_to_php.php should ignore '--plugin' when '--update'
@cigamit cigamit added bug Undesired behaviour resolved A fixed issue labels May 24, 2019
@cigamit
Copy link
Member

cigamit commented May 24, 2019

Should be resolved.

@ddb4github
Copy link
Contributor Author

Should be resolved.

After review my message. I found I make misleading between "Describe the bug" and "Expected behavior".

Actually, current function 'db_update_table' use 'array_diff' to compare PK. so sqltable_to_php.php should always output a array if the latest line of output is db_update_table, like

db_update_table ('plugin_mikrotik_users', $data, false);

So suggest diff as below, base on ad50326:

--- sqltable_to_php.php 2019-05-25 19:42:53.000000000 +0800
+++ sqltable_to_php.php 2019-05-28 13:22:50.264353736 +0800
@@ -155,7 +155,7 @@
                        }

                        if (!empty($pri)) {
-                               if ($plugin != '' || !$create) {
+                               if ($plugin != '' && $create) {
                                        $text .= "\$data['primary'] = '" . implode("`,`", $pri) . "';\n";
                                } else {
                                        $text .= "\$data['primary'] = array('" . implode("','", $pri) . "');\n";

cigamit added a commit that referenced this issue May 28, 2019
Poor logic reported by bug opener.
@cigamit
Copy link
Member

cigamit commented May 28, 2019

Okay, fixed up.

@netniV netniV changed the title sqltable_to_php.php should ignore '--plugin' when '--update' sqltable_to_php.php does not always generate valid table data arrays Jun 2, 2019
@cigamit cigamit closed this as completed Jun 8, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

2 participants