Create bulk insert db function and implement in unix-agent process polling. #1460

Merged
merged 4 commits into from Jul 17, 2015

Projects

None yet

3 participants

@spinza
Contributor
spinza commented Jul 15, 2015

This is a more general version of #1447 . This creates a bulk insert function that could be used elsewhere also.

@f0o f0o and 1 other commented on an outdated diff Jul 16, 2015
includes/dbFacile.php
+ $rowvalues .= ',';
+ }
+ $rowvalues .= "'".mres($value)."'";
+ }
+ $values .= "(".$rowvalues.")";
+ }
+
+ $time_start = microtime(true);
+ $result = dbQuery($sql.$values);
+
+ // logfile($fullSql);
+ $time_end = microtime(true);
+ $db_stats['insert_sec'] += number_format(($time_end - $time_start), 8);
+ $db_stats['insert']++;
+
+ return $id;
@f0o
f0o Jul 16, 2015 Member

$id is never defined.

@spinza
spinza Jul 16, 2015 Contributor

Sorry, left overs from the previous function. Will fix.

@f0o
Member
f0o commented Jul 16, 2015

Commenting minor issue when back from office, please dont merge before.

@f0o f0o commented on the diff Jul 16, 2015
includes/dbFacile.php
+ $rowvalues='';
+ foreach ($row as $key => $value) {
+ if ($rowvalues != '') {
+ $rowvalues .= ',';
+ }
+ $rowvalues .= "'".mres($value)."'";
+ }
+ $values .= "(".$rowvalues.")";
+ }
+
+ $time_start = microtime(true);
+ $result = dbQuery($sql.$values);
+
+ // logfile($fullSql);
+ $time_end = microtime(true);
+ $db_stats['insert_sec'] += number_format(($time_end - $time_start), 8);
@f0o
f0o Jul 16, 2015 Member

number_format() returns string, although php really doesnt care much about it I think round() is more appropriate here

@spinza
spinza Jul 16, 2015 Contributor

number_format() is used like that about 5 or 6 times in dbFacile.php I think it would be silly to have one that's different?

We can do a seperate PR switching them all if you feel the need?

@f0o
f0o Jul 16, 2015 Member

separate PR is good :)

@spinza
spinza Jul 17, 2015 Contributor

Actually $fullsql is also used in every sub in dbFacile.php in similar fashion. Plus all the superfluous comment stuff. I've removed it from dbBulkInsert but really this is a problem with dbFacile.php not with this PR.

@f0o f0o and 1 other commented on an outdated diff Jul 16, 2015
includes/dbFacile.php
+ * Check for boolean false to determine whether insert failed
+ * */
+
+
+function dbBulkInsert($data, $table) {
+ global $fullSql;
+ global $db_stats;
+ // the following block swaps the parameters if they were given in the wrong order.
+ // it allows the method to work for those that would rather it (or expect it to)
+ // follow closer with SQL convention:
+ // insert into the TABLE this DATA
+ if (is_string($data) && is_array($table)) {
+ $tmp = $data;
+ $data = $table;
+ $table = $tmp;
+ // trigger_error('QDB - Parameters passed to insert() were in reverse order, but it has been allowed', E_USER_NOTICE);
@f0o
f0o Jul 16, 2015 Member

I know this part is copy/paste from dbInsert() but I think we can skip this line here to avoid more useless duplication ;)
(Could also apply for Line119 global $fullSql, and Line158?)

@spinza
spinza Jul 16, 2015 Contributor

Will do. Not sure why it had global $fullSql and will remove the // trigger_error comment.

@f0o f0o self-assigned this Jul 16, 2015
@f0o f0o added the Polling label Jul 16, 2015
@spinza spinza Fix return in dbBulkInsert.
c8a5475
@f0o f0o merged commit c2c53fa into librenms:master Jul 17, 2015

2 checks passed

Auto-Deploy Build finished.
Details
Scrutinizer No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment