IDEMPIERE-6133 : Support Export Blob Column for Export SQL Insert Scr…

[uthadehikaru]

…ipts

https://idempiere.atlassian.net/browse/IDEMPIERE-6133

Pull Request Checklist

• My code follows the code guidelines of this project
• My code follows the best practices of this project
• I have performed a self-review of my own code
• My code is easy to understand and review.
• I have checked my code and corrected any misspellings
• In hard-to-understand areas, I have commented my code.
• My changes generate no new warnings

Tests

• I have tested the direct scenario that my code is solving
• I checked all collaterals that can be affected by my changes, and tested other potential affected scenarios
• New and existing unit tests pass locally with my changes
• I have added unit tests that prove my fix is effective or that my feature works

Documentation

• I have made corresponding changes to the documentation as follows:
• • New feature (non-breaking change which adds functionality): I have created the New Feature page in the project wiki explaining the functionality and how to use it. If relevant, I have committed sample data to the core seed to have usable examples in GardenWorld.
• • Breaking change (fix or feature that would cause existing functionality to not work as expected): I have documented the change in a clear way that everyone in the community can understand the impact of the change.
• • Improvement (improves and existing functionality): This documentation is needed if the improvement changes the way the user interacts with the system or the outcome of a process/task changes. If it is just, for instance, a performance improvement, documentation might not be needed.
• The changed/added documentation is in the project wiki (not privately-hosted pdf files or links pointing to a company website) and is complete and self-explanatory.

[hengsin]

The right way is:

• add public String TO_Blob(String hexString) to AdempiereDatabase interface.
• that should return "decode('"+hexString+"','hex')" for DB_PostgreSQL and return "hextoraw('"+hexString+"')" for DB_Oracle.
• Add String database argument to toInsertSQL and buildInsertSQL method
• Inside the buildInsertSQL method, you will call sqlValues.append (DB.getDatabase(database).TO_Blob(Hex.encodeHexString((byte[]) value)));
• For addSQLInsert, String oracle = Database.getDatabase(Database.DB_ORACLE).convertStatement(po.toInsertSQL(Database.DB_Oracle)); and String pg = Database.getDatabase(Database.DB_POSTGRESQL).convertStatement(po.toInsertSQL(Database.DB_POSTGRESQL));

[uthadehikaru]

i see. thx @hengsin for your guideline.

[CarlosRuiz-globalqss]

Something to notice:

My tests in oracle shows that this option is not good, it works just with very little files.

I tested with a small image (406 bytes) and the insert was good.

But when testing with a normal image (170KB), the insert generated is useless, it cannot be used in sqlplus or DBeaver, this is an oracle problem that do not allow too long strings, there are several errors trying to do that:
SP2-0027
SP2-0341
ORA-01489
ORA-01704

So, because of that, I would suggest to have a SysConfig to disable exporting BLOBS in oracle.

[hengsin]

Something to notice:

My tests in oracle shows that this option is not good, it works just with very little files.

I tested with a small image (406 bytes) and the insert was good.

But when testing with a normal image (170KB), the insert generated is useless, it cannot be used in sqlplus or DBeaver, this is an oracle problem that do not allow too long strings, there are several errors trying to do that: SP2-0027 SP2-0341 ORA-01489 ORA-01704

So, because of that, I would suggest to have a SysConfig to disable exporting BLOBS in oracle.

Yes, the size concern is why the original code doesn't export blob.

[hengsin]

As per @CarlosRuiz-globalqss comments on the size issue, I think we should add a sysconfig to make this optional (or a max size of blob that can be export)

[uthadehikaru]

As per @CarlosRuiz-globalqss comments on the size issue, I think we should add a sysconfig to make this optional (or a max size of blob that can be export)

should we add 2 sysconfigs for both databases? like EXPORT_BLOB_COLUMN_FOR_POSTGRESQL and EXPORT_BLOB_COLUMN_FOR_ORACLE and the default is true? what level this sysconfig? per tenant or system? need suggestion for this.

[CarlosRuiz-globalqss]

should we add 2 sysconfigs for both databases? like EXPORT_BLOB_COLUMN_FOR_POSTGRESQL and EXPORT_BLOB_COLUMN_FOR_ORACLE and the default is true? what level this sysconfig? per tenant or system? need suggestion for this.

I think just one SysConfig is OK - EXPORT_BLOB_COLUMN_FOR_INSERT
Configurable per tenant (it requires migration script)
default to Yes
I think is enough to have the option to turn it off for oracle installations when needed.

[CarlosRuiz-globalqss]

Hi @uthadehikaru - can you please apply the following patch:

pr2342PR.patch.txt

Fixes issues I noticed while testing this:

1 - The Test.BinaryData is defined as Image instead of Binary - and it has problems saving records on Test. This must be some regression bug as I remember fixing that sometime ago, but anyways I think binary is more appropriate.

2 - PO.java is trying to save '' for an empty mandatory BLOB, that's not valid in oracle, oracle treats empty strings as null, saving a '0' fixed the issue

3 - in PO.java too found that if you try to save a Broadcast Message while the preference to Log Migration Script is set, then is not possible to save it, the change in line 5426 solves it

cc: @hengsin

[hengsin]

Test.BinaryData doesn't works with Image reference is not a regression. Image reference is for reference to AD_Image and that must be integer instead of blob. If we want to support the use of blob with Image editor, we need to add a new reference for that (for e.g ImageBlob).

[hengsin]

Please apply the following patch to add support for export of large size Oracle blob data:
pr2342-oracle-blob.patch.txt

[hengsin]

tested

[hengsin]

hi @CarlosRuiz-globalqss , the latest update seems ok for me, please review again, thanks.

[CarlosRuiz-globalqss]

tested successfully in postgresql and oracle