Allow passing a table name in GetUpdateSQL()'s $result param

[NathanGibbs3]

Description

Noticed while working on #899 . Both GetInsertSQL() & GetUpdateSQL() create a database-specific SQL statement based on supplied parameters .
GetInsertSQL() can do this using a supplied table name instead of an ADORecordSet object .
GetUpdateSQL() cannot.

Environment

• ADOdb version: < 5.22.7
• Driver or Module: All
• Database type and version: All
• PHP version: All
• Platform: All

• I have tested that the problem is reproducible in the latest release / master branch / hotfix branch

Steps to reproduce

Detailed, step-by-step instructions to reproduce the behavior, including:

• code snippet

$lib = 0; // 0 use 5.22.6 release 1 use snapshot of master branch.
$driver = 'mysqli';

$svr = 'db';
$usr = 'test';
$pwd = 'password';

$DB = 'test';
$tbl = 'test';
$fld = 'test';

if ( $lib == 0 ){
    include('../../ADOdb-5.22.6/adodb.inc.php');
}else{
    include('../../ADOdb/adodb.inc.php');
}

$db = newAdoConnection($driver);
$db->connect($svr, $usr, $pwd, $DB);
print $db->GetInsertSQL($tbl, array($fld=>'test'))."\n";
print $db->GetUpdateSQL($tbl, array($fld=>'What'))."\n";

• SQL to create and populate database objects used by the code snippet

CREATE TABLE test ( test VARCHAR(255) );

Expected behavior

INSERT INTO test ( TEST ) VALUES ( 'test' )
UPDATE test SET TEST='What'

Additional context

#899 Comments thread

I've already implemented a Proof of Concept fix for this, if it's something that would add value to ADOdb, I'll clean up my code and PR it.

[dregad]

By all means do submit your PR, thank you !

If you don't mind, please also provide the relevant update to the documentation. You can do so by adding a code block with revised DokuWiki source in a PR comment, and I'll paste it into the wiki when I merge it.

[NathanGibbs3]

I've been forgetting to ask you about that.
Is there a repo of the documentation source anywhere that could be PRed to?
I'll have a look at the link above and see what I can do.
Just need to learn DocuWiki first. :-)

[dregad]

Is there a repo of the documentation source anywhere that could be PRed to?

No. The documentation is a separate thing, and the wiki is configured for restricted write access

Just need to learn DocuWiki first

Don't sweat it, just type the text and I'll adjust formatting if needed.
For reference if you want to do more: https://adodb.org/dokuwiki/doku.php?id=wiki:syntax

[NathanGibbs3]

I'm not sure what is going on in GetInsertSQL() with $rs->insertSig, $cacheRS, & $cacheCols.
Whatever that code is doing, could it be pulled over into GetUpdateSQL() without breaking anything?
I commented it out in my proof of concept for GetUpdateSQL(), as I have absolutely no idea what that code is doing.
If it is functionality that is useful, I would like to carry it over.

[mnewnham]

I'm not sure what is going on in GetInsertSQL() with $rs->insertSig, $cacheRS, & $cacheCols. Whatever that code is doing, could it be pulled over into GetUpdateSQL() without breaking anything? I commented it out in my proof of concept for GetUpdateSQL(), as I have absolutely no idea what that code is doing. If it is functionality that is useful, I would like to carry it over.

All of the features you name are there to reduce table lookups when doing bulk inserts of records. I added them about 20 years ago and was able to speed up bulk insults by 10x or more. As my companies application can do 250,000 inserts inside a single transaction and I need it done as efficiently and quickly as possible, you can understand my concern at any changes you might be pushing.

As background, what these do is prevent the table schema being read for every insert made by getInsertSql(). This schema read is a resource greedy event and really drags down performance. After being read once they are stored and the in-memory copy re-used.

[NathanGibbs3]

Thank you, this is why I try to ask a million questions before submitting a PR. You & @dregad know this codebase inside & out.
We definitely don't want to slow ADOdb down unnecessarily.

I'm trying to bring the GetInsertSQL() level of functionality over to GetUpdateSQL(), and not break anything in the process.
It definitely sounds like bringing this over into GetUpdateSQL() could speed up bulk updates as well. 👍

For GetupdateSQL(), in the instance where $rs is a table name, it looks like I can just bring what you did in GetInsertSQL() straight over.

In the instance where $rs is an adorecordset, do I need to be doing something different for GetUpdateSQL()?
Should I be using $rs->insertSig & $cacheSig or should I be using something else?

If I can bring it straight over, all need to do is remove the comments from the code below, and we should be in business for `GetUpdateSQL()'

} else if (is_subclass_of($rs, 'adorecordset')) {
//		if (isset($rs->insertSig) && is_integer($cacheRS) && $cacheRS == $rs->insertSig) {
//			$columns = $cacheCols;
//		} else {
			$columns = [];
			for ($i=0, $max=$rs->FieldCount(); $i < $max; $i++)
				$columns[] = $rs->FetchField($i);
//			$cacheRS = $cacheSig;
//			$cacheCols = $columns;
//			$rs->insertSig = $cacheSig++;
//		}

As stated, I'm trying not to break anything in the process. 😄

[NathanGibbs3]

In the instance where $rs is an adorecordset, do I need to be doing something different for GetUpdateSQL()?
Should I be using $rs->insertSig & $cacheSig or should I be using something else?

Disregard that question, it helps when I scroll up in the code and read the first few lines of _adodb_getinsertsql() 🤦‍♂️
I can bring what you did there straight into _adodb_getupdatesql()

[NathanGibbs3]

Actually part of that question still stands.

Should I be using $rs->insertSig?

or
should I create a new $updateSig public var in the ADORecordSet class to keep GetInsertSQL() and GetUpdateSQL() from stepping on each other?

[NathanGibbs3]

Going with keep GetInsertSQL() and GetUpdateSQL() from stepping on each other.
I plan to PR this at the end of the week, if you all are OK with it.
Let me know if I've missed anything, otherwise, we will catch it in the PR.

@dregad , I didn't forget your request for updated documentation, I'll send that in the PR comment.