Reimplement system triggers in C++/GDML code #8202

asfernandes · 2024-08-02T01:52:08Z

No description provided.

hvlad · 2024-08-02T06:51:55Z

builds/win32/preprocess.bat

@@ -78,7 +78,7 @@ goto :EOF
 @for %%i in (metd) do @call :PREPROCESS dsql %%i -gds_cxx
 @for %%i in (DdlNodes, PackageNodes) do @call :PREPROCESS dsql %%i -gds_cxx
 @for %%i in (gpre_meta) do @call :PREPROCESS gpre/std %%i
-@for %%i in (dfw, dpm, dyn_util, fun, grant, ini, met, scl, Function) do @call :PREPROCESS jrd %%i -gds_cxx
+@for %%i in (dfw, dpm, dyn_util, fun, grant, ini, met, scl, Function, SystemTriggers) do @call :PREPROCESS jrd %%i -gds_cxx


It should be in BOOT_PROCESS section (above) too

hvlad · 2024-08-02T06:54:08Z

src/jrd/exe.cpp

+				}
+			}
+		}
+	}


Consider to move this block into separate routine - SystemTriggers::execute(), for example

I did it with multiple execute per moment/type, letting EXE still choosing them.

aafemt · 2024-08-03T09:33:19Z

src/jrd/SystemTriggers.epp

+namespace
+{
+
+void beforeDeleteCheckConstraint(thread_db* tdbb, Record* record)


It is better to name routines according to what they do instead of when they are called.

…ers.

asfernandes · 2024-08-04T01:47:20Z

I have validated this branch with firebird-qa and the differences with master are:

tests.bugs.core_4731_test: with firebird-qa inserts records in rdb$types with rdb$system_flag (not nullable) set to null and expect the records there. Firebird was wrong due to not create rdb$runtime field for it due to no system triggers for this table - now fixed in Firebird code.

tests.bugs.core_4725_test
tests.functional.gtcs.test_ref_integ_drop_fk_index
tests.functional.gtcs.test_ref_integ_drop_pk_constraint
tests.functional.gtcs.test_ref_integ_drop_pk_index
tests.functional.gtcs.test_ref_integ_inactive_fk_index
tests.functional.gtcs.test_ref_integ_inactive_pk_index
tests.functional.gtcs.test_ref_integ_inactive_pk_index_2
tests.functional.index.alter.test_04
tests.functional.index.alter.test_05

These tests excepts in some operations SQLSTATE = 27000 but now Firebird returns 42000. The change is correct as 27000 is related to the error "action cancelled by system trigger" and not the actual SQLSTATE associated with the error.

CC @pavel-zotov

aafemt · 2024-08-04T10:32:46Z

src/jrd/exe_proto.h

+
+	private:
+		unsigned id;
+		static inline std::atomic<unsigned> generator;


The cache is bound to attachment and not database, what is the point for atomic generator here? BTW, std::atomic_uint is defined.

pavel-zotov · 2024-08-06T22:47:56Z

tests.bugs.core_4731_test: with firebird-qa inserts records ... Firebird was wrong ... now fixed in Firebird code.

I see result = 'passed' for this test on 6.0.0.419-3505a5ec and 6.0.0.421-ed60d4d. Nothing to do with it ?

Following:

tests.bugs.core_4725_test
tests.functional.gtcs.test_ref_integ_drop_fk_index
. . .
tests.functional.index.alter.test_04
tests.functional.index.alter.test_05
These tests excepts in some operations SQLSTATE = 27000 but now Firebird returns 42000

-- have been adjusted with current output in FB 6.x

asfernandes · 2024-08-07T00:49:43Z

tests.bugs.core_4731_test: with firebird-qa inserts records ... Firebird was wrong ... now fixed in Firebird code.

I see result = 'passed' for this test on 6.0.0.419-3505a5ec and 6.0.0.421-ed60d4d. Nothing to do with it ?

Without changes for me:

E       AssertionError: assert   
E           -- Executed with role: NONE. Expressions that passes WITHOUT errors:
E           -- count_of_passed:             0
E           -- gdscode list for blocked:    335544926
E           -- Executed with role: RDB$ADMIN. Expressions that passes WITHOUT errors:
E         - -- count_of_passed:             24
E         ?                                 ^\n
E         + -- count_of_passed:             14
E         ?                                 ^\n
E           VULNERABLE_EXPR                 delete from RDB$BACKUP_HISTORY t  rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 insert into RDB$BACKUP_HISTORY(RDB$BACKUP_ID , RDB$TIMESTAMP , RDB$BACKUP_LEVEL , RDB$GUID , RDB$SCN , RDB$FILE_NAME) values(null, null, null, null, null, null) returning rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 delete from RDB$DB_CREATORS t  rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 insert into RDB$DB_CREATORS(RDB$USER , RDB$USER_TYPE) values(null, null) returning rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$DB_CREATORS t  set t.RDB$USER = 'C'  rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$DB_CREATORS t  set t.RDB$USER = null  rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$DB_CREATORS t  set t.RDB$USER_TYPE = 32767  rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$DB_CREATORS t  set t.RDB$USER_TYPE = null  rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$FUNCTIONS t  set t.RDB$FUNCTION_SOURCE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$PACKAGES t  set t.RDB$PACKAGE_BODY_SOURCE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$PACKAGES t  set t.RDB$PACKAGE_HEADER_SOURCE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$PROCEDURES t  set t.RDB$PROCEDURE_SOURCE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$RELATIONS t  set t.RDB$VIEW_SOURCE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           VULNERABLE_EXPR                 update RDB$TRIGGERS t  set t.RDB$TRIGGER_SOURCE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 insert into RDB$TYPES(RDB$FIELD_NAME , RDB$TYPE , RDB$TYPE_NAME , RDB$DESCRIPTION , RDB$SYSTEM_FLAG) values(null, null, null, null, null) returning rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$DESCRIPTION = 'test_for_blob' where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$DESCRIPTION = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$FIELD_NAME = 'C' where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$FIELD_NAME = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$SYSTEM_FLAG = 32767 where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$TYPE = 32767 where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$TYPE = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$TYPE_NAME = 'C' where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E         - VULNERABLE_EXPR                 update RDB$TYPES t  set t.RDB$TYPE_NAME = null where coalesce(rdb$system_flag,0)=0 rows 1 returning t.rdb$db_key; -- length of returned rdb$dbkey=8
E           -- gdscode list for blocked:    335544926

pavel-zotov · 2024-08-08T04:55:20Z

Without changes for me:
...
E - -- count_of_passed: 24
E + -- count_of_passed: 14

For which snapshot number this test issued "count_of_passed: 14" ?
I still see old (24):

Used config:

AuthServer = Srp, Win_Sspi, Legacy_Auth
BugCheckAbort = 1
ClearGTTAtRetaining = 0
ClientBatchBuffer = 131072
ConnectionIdleTimeout = 0
DefaultDBCachePages = 10K
ExtConnPoolLifeTime = 10
ExtConnPoolSize = 10
ExternalFileAccess = Full
InlineSortThreshold = 1000
IpcName = xnet_fb6x_ss
KeyHolderPlugin = fbSampleKeyHolder
MaxIdentifierByteLength = 252
MaxIdentifierCharLength = 63
MaxParallelWorkers = 8
MaxUnflushedWrites = -1
MaxUnflushedWriteTime = -1
ParallelWorkers = 1
ReadConsistency = 0
RemoteServicePort = 3600
ServerMode = Super
SnapshotsMemSize = 64K
StatementTimeout = 300
TempCacheLimit = 1G
TipCacheBlockSize = 4M
UseFileSystemCache = true
UserManager = Srp, Legacy_UserManager
WireCrypt = Enabled
WireCryptPlugin = ChaCha, Arc4

Also, no problem with this test on firebirdtest.com page:

pavel-zotov · 2024-08-08T06:17:02Z

I still see old (24):

Sorry, i've found the reason.
Will fix that.

asfernandes force-pushed the work/system-triggers branch from 53d6a8e to 9e373d0 Compare August 2, 2024 02:06

hvlad reviewed Aug 2, 2024

View reviewed changes

aafemt reviewed Aug 3, 2024

View reviewed changes

asfernandes added 7 commits August 3, 2024 15:38

Regenerate system triggers improving formatting and constant names.

0cc8de3

Add CachedRequestId mechanism for internal request cache.

2c48a4e

Reimplement system triggers in C++/GDML code.

9926ab8

Preprocess SystemTriggers in windows build boot phase.

ff16556

Replace individual public functions by moment/type execute categories.

f947d0b

Correction for drop not null constraint validation.

19823f8

Correction: disallow updates in RDB$USER_PRIVILEGES.

31780d5

asfernandes force-pushed the work/system-triggers branch from 320280c to 31780d5 Compare August 3, 2024 18:39

Correction: revert code related with WITH CHECK OPTION internal trigg…

aaf0e33

…ers.

aafemt reviewed Aug 4, 2024

View reviewed changes

Misc.

e9410dd

asfernandes merged commit 16da398 into master Aug 5, 2024
48 checks passed

asfernandes deleted the work/system-triggers branch August 5, 2024 10:44

asfernandes added a commit that referenced this pull request Aug 6, 2024

Correction for #8202.

039c7e3

asfernandes added a commit to FirebirdSQL/fbtcs that referenced this pull request Aug 6, 2024

Update test results after FirebirdSQL/firebird#8202.

0a2c466

dyemanov pushed a commit that referenced this pull request Nov 16, 2024

Correction for #8202.

d670ad6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Reimplement system triggers in C++/GDML code #8202

Reimplement system triggers in C++/GDML code #8202

Uh oh!

asfernandes commented Aug 2, 2024

Uh oh!

hvlad Aug 2, 2024

Uh oh!

hvlad Aug 2, 2024

Uh oh!

asfernandes Aug 2, 2024

Uh oh!

aafemt Aug 3, 2024

Uh oh!

asfernandes commented Aug 4, 2024

Uh oh!

aafemt Aug 4, 2024

Uh oh!

Uh oh!

pavel-zotov commented Aug 6, 2024

Uh oh!

asfernandes commented Aug 7, 2024

Uh oh!

pavel-zotov commented Aug 8, 2024

Uh oh!

pavel-zotov commented Aug 8, 2024

Uh oh!

Uh oh!

Uh oh!

Reimplement system triggers in C++/GDML code #8202

Reimplement system triggers in C++/GDML code #8202

Uh oh!

Conversation

asfernandes commented Aug 2, 2024

Uh oh!

hvlad Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

hvlad Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

asfernandes Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

aafemt Aug 3, 2024

Choose a reason for hiding this comment

Uh oh!

asfernandes commented Aug 4, 2024

Uh oh!

aafemt Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pavel-zotov commented Aug 6, 2024

Uh oh!

asfernandes commented Aug 7, 2024

Uh oh!

pavel-zotov commented Aug 8, 2024

Uh oh!

pavel-zotov commented Aug 8, 2024

Uh oh!

Uh oh!