Reverse engineering existing database : triggers in its own folder
paulkatich opened this issue · 18 comments
I am using the obevo cli for reverse engineering on oracle database. Currently the reveng tool puts all the triggers and tables in the same file. What is Obevo recommendation on this ? Is it possible to separate the triggers in to separate files by table ? We have a legacy database which lot of DDL for tables with indices. By separating triggers in a separate file will it cause any issues with changes ?
A use case for our database is we have logon triggers which sets the session specific configs. So this dont belong to a table.
We do allow triggers in separate files in their own folder named /trigger. It is not enabled by default for all dbms types; I'll have to enable it for Oracle.
Assuming the trigger does belong to a schema or user, the existing logic should work for it. If not, then we need a little more work on it, but no problem to do
Btw, is the ask here the same as your last question in #219? If so, I'll close that ticket
Yes same as in #219
Fix committed - you can give this snapshot a try - https://oss.sonatype.org/content/repositories/snapshots/com/goldmansachs/obevo/obevo-cli/master-SNAPSHOT/obevo-cli-master-20190501.001845-17-dist.zip
(This is the same link as for your other ticket #231)
FYI on the trigger code - you may want to leverage the in-built tokens to specify the schema of your trigger, in case the Sql actually requires it. For an example, see this code:
We had good improvemnts. But looks like we missed a few.
EX:
//// CHANGE name=change0
CREATE TABLE FP_SALE_TYPES
( SALE_TYPE_ID VARCHAR2(20) NOT NULL ENABLE,
NAME VARCHAR2(30) NOT NULL ENABLE,
DEFAULT_OPEN_SALE_IND VARCHAR2(1) DEFAULT 'Y',
DESCRIPTION VARCHAR2(255) NOT NULL ENABLE,
CREATED_DTM DATE DEFAULT sysdate,
CREATED_BY VARCHAR2(30) DEFAULT user,
CHANGED_DTM DATE DEFAULT sysdate,
CHANGED_BY VARCHAR2(30) DEFAULT user,
CREATED_BY_LE_ID NUMBER(36,0),
CREATED_BY_OPERATING_AS_LE_ID NUMBER(36,0),
CREATED_BY_ORG_UNIT_LE_ID NUMBER(36,0),
CONSTRAINT STYP_PK PRIMARY KEY (SALE_TYPE_ID)
USING INDEX PCTFREE 10 INITRANS 2 MAXTRANS 255 COMPUTE STATISTICS
TABLESPACE XCOSX_SMALL_01 ENABLE,
CONSTRAINT AVCON_850217_DEFAU_000 CHECK (DEFAULT_OPEN_SALE_IND IN ('Y', 'N')) ENABLE
) SEGMENT CREATION IMMEDIATE
PCTFREE 10 PCTUSED 40 INITRANS 1 MAXTRANS 255
NOCOMPRESS LOGGING
TABLESPACE XCOSD
GO
//// CHANGE name=change1
CREATE OR REPLACE FORCE EDITIONABLE EDITIONING VIEW "ATLANTIS"."FP_SALE_TYPES#" ("SALE_TYPE_ID", "NAME", "DEFAULT_OPEN_SALE_IND", "DESCRIPTION", "CREATED_DTM", "CREATED_BY", "CHANGED_DTM", "CHANGED_BY", "CREATED_BY_LE_ID", "CREATED_BY_OPERATING_AS_LE_ID", "CREATED_BY_ORG_UNIT_LE_ID") AS
select SALE_TYPE_ID SALE_TYPE_ID, NAME NAME, DEFAULT_OPEN_SALE_IND DEFAULT_OPEN_SALE_IND, DESCRIPTION DESCRIPTION, CREATED_DTM CREATED_DTM, CREATED_BY CREATED_BY, CHANGED_DTM CHANGED_DTM, CHANGED_BY CHANGED_BY, CREATED_BY_LE_ID CREATED_BY_LE_ID, CREATED_BY_OPERATING_AS_LE_ID CREATED_BY_OPERATING_AS_LE_ID, CREATED_BY_ORG_UNIT_LE_ID CREATED_BY_ORG_UNIT_LE_ID from FP_SALE_TYPES
GO
//// CHANGE name=change2
CREATE OR REPLACE EDITIONABLE TRIGGER "ATLANTIS"."FP_SCOA_AUD_TRG"
BEFORE INSERT OR UPDATE
ON "ATLANTIS"."FP_SCORE_ANSWERS#" FOR EACH ROW
BEGIN
DECLARE
LOGON_RECORD FP_PKG_SEC_SECURITY.T_LOGON_RECORD;
BEGIN
/* populate auditing columns */
IF INSERTING THEN
:NEW.CREATED_BY := USER;
:NEW.CHANGED_BY := USER;
/* If the date created has been specified as part of the INSERT
then use that date for created by and changed by
otherwise set the values for both columns to todays date
*/
IF :NEW.CREATED_DTM IS NOT NULL THEN
:NEW.CHANGED_DTM := :NEW.CREATED_DTM;
ELSE
:NEW.CREATED_DTM := SYSDATE;
:NEW.CHANGED_DTM := SYSDATE;
END IF;
/* the following code was added by SHOLLOWS to support */
/* the storing of additional auditing data for each record */
LOGON_RECORD := fp_pkg_sec_security.get_logon;
:NEW.CREATED_BY_LE_ID := LOGON_RECORD.logon_user_le_id;
:NEW.CREATED_BY_OPERATING_AS_LE_ID := LOGON_RECORD.operating_as_le_id;
:NEW.CREATED_BY_ORG_UNIT_LE_ID := LOGON_RECORD.Trans_Org_Unit_le_id;
ELSIF UPDATING THEN
:NEW.CHANGED_BY := USER;
IF :NEW.CHANGED_DTM = :OLD.CHANGED_DTM
OR :NEW.CHANGED_DTM IS NULL THEN
:NEW.CHANGED_DTM := SYSDATE;
END IF;
END IF;
END;
END FP_SCOA_AUD_TRG;
/
ALTER TRIGGER "ATLANTIS"."FP_SCOA_AUD_TRG" ENABLE
GO
//// CHANGE name=change3
CREATE OR REPLACE EDITIONABLE TRIGGER "ATLANTIS"."FP_SCOC_AUD_TRG"
BEFORE INSERT OR UPDATE
ON "ATLANTIS"."FP_SCORE_CHOICES#" FOR EACH ROW
BEGIN
DECLARE
LOGON_RECORD FP_PKG_SEC_SECURITY.T_LOGON_RECORD;
BEGIN
/* populate auditing columns */
IF INSERTING THEN
:NEW.CREATED_BY := USER;
:NEW.CHANGED_BY := USER;
/* If the date created has been specified as part of the INSERT
then use that date for created by and changed by
otherwise set the values for both columns to todays date
*/
IF :NEW.CREATED_DTM IS NOT NULL THEN
:NEW.CHANGED_DTM := :NEW.CREATED_DTM;
ELSE
:NEW.CREATED_DTM := SYSDATE;
:NEW.CHANGED_DTM := SYSDATE;
END IF;
/* the following code was added by SHOLLOWS to support */
/* the storing of additional auditing data for each record */
LOGON_RECORD := fp_pkg_sec_security.get_logon;
:NEW.CREATED_BY_LE_ID := LOGON_RECORD.logon_user_le_id;
:NEW.CREATED_BY_OPERATING_AS_LE_ID := LOGON_RECORD.operating_as_le_id;
:NEW.CREATED_BY_ORG_UNIT_LE_ID := LOGON_RECORD.Trans_Org_Unit_le_id;
ELSIF UPDATING THEN
:NEW.CHANGED_BY := USER;
IF :NEW.CHANGED_DTM = :OLD.CHANGED_DTM
OR :NEW.CHANGED_DTM IS NULL THEN
:NEW.CHANGED_DTM := SYSDATE;
END IF;
END IF;
END;
END FP_SCOC_AUD_TRG;
/
ALTER TRIGGER "ATLANTIS"."FP_SCOC_AUD_TRG" ENABLE
GO
//// CHANGE INDEX name=STYP_PK
CREATE UNIQUE INDEX STYP_PK ON FP_SALE_TYPES (SALE_TYPE_ID)
PCTFREE 10 INITRANS 2 MAXTRANS 255 COMPUTE STATISTICS
TABLESPACE XCOSX_SMALL_01
GO
//// CHANGE name=change4
CREATE INDEX "ATLANTIS"."TED_IX1" ON OPEN_FEVE ("FINANCIAL_EVENT_ID")
PCTFREE 10 INITRANS 2 MAXTRANS 255 COMPUTE STATISTICS
TABLESPACE "XCOSD"
PARALLEL 4
GO
Is that all in the same reverse-engineered file? I'm surprised to see the trigger and the indexes in the table file
If it is not a problem, can you send over the interim/output.sql file that the reverse-engineering generates? That would be the ideal test case for me. However, in case that is not possible (e.g. if you have any sensitive content in that file), I should be able to work with the sample you provided here
@shantstepanian can I email you ?
You can get my email from here - https://github.com/goldmansachs/obevo/blob/master/Obevo_Javasig.pdf
@shantstepanian sorry. I thought I can email. Our info sec team has some issues with this. Can you please work with the example above.
Hi, please try this snapshot
https://github.com/goldmansachs/obevo/releases/download/untagged-c71181f1029ffb1193da/obevo-cli-master-SNAPSHOT-dist.zip
I added a minor fix around the view names that may be throwing things off; it may or may not work.
If it does not, please execute the NEWREVENG command with this flag added: -debugLogEnabled
This will add additional information to your output to help analyze what is going on.
Thanks for your patience with this as we iron out these issues. With the new debugLogEnabled feature, we hopefully won't need as many iterations like this to resolve issues
@shantstepanian I am getting file not found when I try to download the file.
Sorry, try this link - https://github.com/goldmansachs/obevo/releases/download/master-SNAPSHOT/obevo-cli-master-SNAPSHOT-dist.zip
It should also be available from the Releases page (master-SNAPSHOT)
The link doesnt work. I went to releases pages. I only get the source code but not jars. Can you please provide a link with the cli jar files ?
Apologies again - the link should be fine now, and the release page corrected
https://github.com/goldmansachs/obevo/releases/download/master-SNAPSHOT/obevo-cli-master-SNAPSHOT-dist.zip
Worst-case, try this link: https://oss.sonatype.org/content/repositories/snapshots/com/goldmansachs/obevo/obevo-cli/master-SNAPSHOT/obevo-cli-master-20190506.220347-19-dist.zip
Got it. @shantstepanian I am still seeing few examples where the trigger is in a separate file. I added the debug enabled. Can I know where is the diagnostic information ? Is it in the output.sql in interim folder ?
Clarifying the requirement - I do expect that triggers should be in a separate folder separate from the table file (as per your original requirement around the login triggers not belonging to a table). This is how Obevo currently works (i.e. no distinguishing between triggers belonging to a table and triggers that don't). We have it as a separate file as it allows us to modify the trigger file in place (as of today, any //// CHANGE in a table file cannot be edited in place, which makes it more cumbersome to manage triggers there)
I would not rule out a future enhancement to allow triggers to be defined either way (e.g. co-located with the table file vs. in a separate file entirely, and to remove the constraint mentioned above). We can create this under a separate issue if you'd like, though it is a larger change and thus won't happen in the near-term.
In terms of the debug information, that shows up as a line like so within the file:
-- DEBUG COMMENT: secondaryName=null; newPatternMatch=true; objectType=FUNCTION; originalObjectName=SP3
This information is more useful for the index issues you saw in your other ticket, where indices were not getting grouped to the correct file. I'll post more comments on that ticket
Just wanted to check if this is still an issue, after the rerun that you did in issue #231?
Closing this ticket. Let us know if you face further issues on this issue