-
-
Notifications
You must be signed in to change notification settings - Fork 300
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
IDEMPIERE-6133 : Support Export Blob Column for Export SQL Insert Scr… #2342
IDEMPIERE-6133 : Support Export Blob Column for Export SQL Insert Scr… #2342
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The right way is:
- add
public String TO_Blob(String hexString)
toAdempiereDatabase
interface. - that should return
"decode('"+hexString+"','hex')"
for DB_PostgreSQL and return"hextoraw('"+hexString+"')"
for DB_Oracle. - Add
String database
argument totoInsertSQL
andbuildInsertSQL
method - Inside the
buildInsertSQL
method, you will callsqlValues.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));
andString pg = Database.getDatabase(Database.DB_POSTGRESQL).convertStatement(po.toInsertSQL(Database.DB_POSTGRESQL));
i see. thx @hengsin for your guideline. |
org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java
Outdated
Show resolved
Hide resolved
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: 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java
Outdated
Show resolved
Hide resolved
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 |
- use java17 hex string format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @uthadehikaru - can you please apply the following patch:
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
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). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please apply the following patch to add support for export of large size Oracle blob data:
pr2342-oracle-blob.patch.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested
hi @CarlosRuiz-globalqss , the latest update seems ok for me, please review again, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested successfully in postgresql and oracle
…ipts
https://idempiere.atlassian.net/browse/IDEMPIERE-6133
Pull Request Checklist
Tests
Documentation