pradosoft / prado

Prado - Component Framework for PHP
Other
187 stars 70 forks source link

#940 TApplicationComponent::getClassFxEvents update and TDBCache::setValue uses atomic SQL #952

Closed belisoful closed 1 year ago

belisoful commented 1 year ago

I have been testing the SQL with SqlLite3 and it works. From what I know, this should work in MySQL and other databases as well, as REPLACE is (edit: NOT) part of SQL common language.

The update to getClassFxEvents is also a win. I think it's at least a little faster.

I'm doing further testing on this after a good nights rest.

ctrlaltca commented 1 year ago

I'm not sure REPLACE is standard across all the DMBS. It surely works in MySQL/MariaDB and Sqlite. Other DBMS use UPSERT (postgres), MERGE (Oracle, DB2, M$ Sql Server) We could introduce a switch on $db->getDriverName()implementing the use of REPLACE for mysql and sqlite, and keeping the old behavior for the other drivers (until somebody else will need and implement a better version for the other drivers, too).

belisoful commented 1 year ago

@ctrlaltca I was actually getting to the point of doing implementation specific SQL before sleep took over last night. I'm looking into MERGE for multiple platforms.

Importantly. In looking at getClassFxEvents, the TDBCache is loaded after the primary use of the function. Put another way, TDBCache is not restoring the cache on load because the TDBCache is not loaded. This is leading to more processing time in this function than needed.

I'm thinking of removing TDBCache from the function and replace it with an exclusive lock write file in runtime. On load, it reads the runtime fxEvents cache file. If there is a new class that is loaded, it gets saved to the file. If it can't get a lock on the file, writing is skipped.

belisoful commented 1 year ago

Here is the system dependent SQL I have so far:

if (in_array($driver, ['mysql', 'mysqli', 'sqlite', 'sqlite2', 'ibm', 'oci', 'sqlsrv', 'mssql', 'dblib', 'pgsql'])) {
if (in_array($driver, ['mysql', 'mysqli', 'sqlite', 'sqlite2']))
    $sql = "REPLACE INTO {$this->_cacheTable} (itemkey,value,expire) VALUES (:key,:value,$expire)";
elseif ($driver === 'pgsql') {
    $sql = "INSERT INTO {$this->_cacheTable} (itemkey, value, expire) VALUES (:key, :value, :expire) ".
        "ON CONFLICT (itemkey) DO UPDATE SET value = EXCLUDED.value, expire = EXCLUDED.expire";
} else {
    $sql = "MERGE INTO {$this->_cacheTable} AS c " .
        "USING (SELECT :key AS itemkey, :value AS value, $expire AS expire) AS data " .
        "ON c.itemkey = data.itemkey " .
        "WHEN MATCHED THEN " .
        "UPDATE SET c.value = data.value, c.expire = data.expire " .
        "WHEN NOT MATCHED THEN " .
        "INSERT (itemkey, value, expire) " .
        "VALUES (data.itemkey, data.value, data.expire)";
}

I used ChatGPT to convert the SQL between systems and studying it and web resources: this seem correct.

I understand your suggestion that someone with these systems to test should be the ones to write it, as they can test it. BIG however....

There are other ways of validating this SQL though. ChatGPT can be asked to "simulate" (by "being") these environments and run these sql statements. What a world we live in today!!!

The ChatGPT prompt is "Pretend you are a PostGres Database and you accept SQL statements and run them as if you were a Postgres database; you return the result like a postgres database"

Then feed it the following: CREATE TABLE pradocache (itemkey CHAR(128) PRIMARY KEY, value BYTEA, expire INTEGER) INSERT INTO pradocache VALUES ('key', 'data', 0) and finally INSERT INTO pradocache (itemkey, value, expire) VALUES ('key', 'replaced data', 1) ON CONFLICT (itemkey) DO UPDATE SET value = EXCLUDED.value, expire = EXCLUDED.expire INSERT INTO pradocache (itemkey, value, expire) VALUES ('newkey', 'inserted data', 1) ON CONFLICT (itemkey) DO UPDATE SET value = EXCLUDED.value, expire = EXCLUDED.expire

and to check Select * from pradocache;

I'm about to do the same for the "MERGE" sql to validate it.

The only issue I am running into is some confusion around the DriverName for DB2. LOL. ChatGPT says its "ibm" then "ibm_db2" then "ibm-db2" then "ibm". Do we have any clarity on the Driver Name for IBM's DB2?

Also, we should have Constants for each driver name. It's used enough in enough places that, to reduce errors, we should have these be constants somewhere.

belisoful commented 1 year ago

Using ChatGPT as an oracle database simulator, the MERGE Sql does in fact work properly as expected.

belisoful commented 1 year ago

Tested in "chatGPT MSSql simulator" as well and works.

belisoful commented 1 year ago

I have committed the new update to TDBCache::setValue in this merge. You can review that aspect now. (though do not merge the pull request yet).

I am still working on the implementation of getClassFXEvents that uses a runtime file cache rather than TDBCache. I'm running into issues with the permission manager being loaded twice. ????? !???

belisoful commented 1 year ago

The new getClassFXEvents is here that use a file cache in the "runtime" folder. This loads before the TDBCache module loads, thankfully. It should be concurrency safe, as far as I know.

belisoful commented 1 year ago

Before merging this PR, I'd like confirmation that the DB2 Driver Name is "ibm"

ctrlaltca commented 1 year ago

The only issue I am running into is some confusion around the DriverName for DB2. LOL. ChatGPT says its "ibm" then "ibm_db2" then "ibm-db2" then "ibm". Do we have any clarity on the Driver Name for IBM's DB2?

The last time i used it was "ibm", luckily i don't have to mess with those systems anymore.