Fixed the major part of the slowdown in CORE-2827: Very slow prepare for complex interrelated metadata with many triggers indirectly involved in the operation being prepared.

Code cleanup.
2025-01-24 09:23:03 +01:00 · 2010-10-13 12:11:52 +00:00 · 2010-10-13 12:11:52 +00:00 · 24777be976
commit 24777be976
parent 28a44566d7
12 changed files with 34 additions and 179 deletions
--- a/src/jrd/blb.cpp
+++ b/src/jrd/blb.cpp
@ -1656,7 +1656,7 @@ void BLB_put_slice(	thread_db*	tdbb,

 	SSHORT	n;
 	if (info.sdl_info_field.length()) {
-	    n = MET_lookup_field(tdbb, relation, info.sdl_info_field, 0);
+	    n = MET_lookup_field(tdbb, relation, info.sdl_info_field);
 	}
 	else {
 		n = info.sdl_info_fid;
--- a/src/jrd/cmp.cpp
+++ b/src/jrd/cmp.cpp
@ -379,9 +379,7 @@ static void verify_trigger_access(thread_db* tdbb, jrd_rel* owner_relation, trig
 					continue;
 				}
 				if (!strcmp(access->acc_type, object_column) &&
-					(MET_lookup_field(tdbb, owner_relation, access->acc_name,
-						&access->acc_security_name) >= 0 ||
-					 MET_relation_default_class(tdbb, owner_relation->rel_name, access->acc_security_name)))
+					(owner_relation->rel_name == access->acc_r_name))
 				{
 					continue;
 				}
@ -2293,12 +2291,10 @@ void CMP_post_access(thread_db* tdbb,

 	size_t i;

-	if (csb->csb_access.find(access, i))
+	if (!csb->csb_access.find(access, i))
 	{
-		return;
+		csb->csb_access.insert(i, access);
 	}
-
-	csb->csb_access.insert(i, access);
 }


@ -6119,7 +6115,7 @@ static void post_procedure_access(thread_db* tdbb, CompilerScratch* csb, jrd_prc

 	// this request must have EXECUTE permission on the stored procedure
 	CMP_post_access(tdbb, csb, prc_sec_name, 0, SCL_execute, object_procedure,
-					procedure->prc_name.c_str());
+					procedure->prc_name);

 	// Add the procedure to list of external objects accessed
 	ExternalAccess temp(procedure->prc_id);
--- a/src/jrd/cmp_proto.h
+++ b/src/jrd/cmp_proto.h
@ -49,17 +49,8 @@ Jrd::jrd_nod* CMP_pass2(Jrd::thread_db* tdbb, Jrd::CompilerScratch* csb, Jrd::jr
 	Jrd::jrd_nod* parent);

 void CMP_post_access(Jrd::thread_db*, Jrd::CompilerScratch*, const Firebird::MetaName&, SLONG,
-					 Jrd::SecurityClass::flags_t, const TEXT*, const Firebird::MetaName&, const Firebird::MetaName&);
-inline void CMP_post_access(Jrd::thread_db* tdbb,
-							Jrd::CompilerScratch* csb,
-							const Firebird::MetaName& security_name,
-							SLONG view_id,
-							Jrd::SecurityClass::flags_t mask,
-							const TEXT* type_name,
-							const Firebird::MetaName& name)
-{
-	CMP_post_access(tdbb, csb, security_name, view_id, mask, type_name, name, "");
-}
+					 Jrd::SecurityClass::flags_t, const TEXT*,
+					 const Firebird::MetaName&, const Firebird::MetaName& = "");

 void CMP_post_resource(Jrd::ResourceList*, void*, Jrd::Resource::rsc_s, USHORT);
 void CMP_release(Jrd::thread_db*, Jrd::jrd_req*);
--- a/src/jrd/dfw.epp
+++ b/src/jrd/dfw.epp
@ -3997,7 +3997,7 @@ static bool delete_rfr(thread_db* tdbb, SSHORT phase, DeferredWork* work,
 		relation = MET_lookup_relation_id(tdbb, work->dfw_id, false);
 		if (relation)
 		{
-			const int id = MET_lookup_field(tdbb, relation, work->dfw_name, 0);
+			const int id = MET_lookup_field(tdbb, relation, work->dfw_name);
 			if (id >= 0)
 			{
 				vec<jrd_fld*>* vector = relation->rel_fields;
--- a/src/jrd/irq.h
+++ b/src/jrd/irq.h
@ -110,7 +110,6 @@ enum irq_type_t
 	irq_format6,			// make a new format for a record
 	irq_r_gen_id_num,		// lookup generator by ID.
 	irq_verify_role_name,	// ensure role exists in roles & user_privileges.
-	irq_l_relation_defsec,	// check the default sec class name against rel.
 	irq_m_index_seg,		// modify per-segment index selectivity

 	irq_l_subtype,			// lookup subtype (charset/collation)
--- a/src/jrd/met.epp
+++ b/src/jrd/met.epp
@ -1893,8 +1893,7 @@ SLONG MET_lookup_exception_number(thread_db* tdbb, const Firebird::MetaName& nam

 int MET_lookup_field(thread_db* tdbb,
 					 jrd_rel* relation,
-					 const Firebird::MetaName& name,
-					 const Firebird::MetaName* security_name)
+					 const Firebird::MetaName& name)
 {
 /**************************************
 *
@ -1904,9 +1903,6 @@ int MET_lookup_field(thread_db* tdbb,
 *
 * Functional description
 *      Look up a field name.
- *	Additionally, if security_name is a not null pointer,
- *	it's used to include the condition that it should match
- *	the field's security class name, too.
 *
 *	if the field is not found return -1
 *
@ -1930,14 +1926,7 @@ int MET_lookup_field(thread_db* tdbb,
 				jrd_fld* field = *fieldIter;
 				if (field->fld_name == name)
 				{
-					if (!security_name)
-					{
-						return id;
-					}
-					if (field->fld_security_name == *security_name)
-					{
-						return id;
-					}
+					return id;
 				}
 			}
 		}
@ -1964,15 +1953,7 @@ int MET_lookup_field(thread_db* tdbb,
 			REQUEST(irq_l_field) = request;
 		}

-		if (!security_name) {
-			id = X.RDB$FIELD_ID;
-		}
-		else {
-			if ((!X.RDB$SECURITY_CLASS.NULL) && (*security_name == X.RDB$SECURITY_CLASS))
-			{
-				id = X.RDB$FIELD_ID;
-			}
-		}
+		id = X.RDB$FIELD_ID;

 	END_FOR;

@ -3415,52 +3396,6 @@ jrd_rel* MET_relation(thread_db* tdbb, USHORT id)
 }


-bool MET_relation_default_class(thread_db* tdbb, const Firebird::MetaName& relation_name,
-								const Firebird::MetaName& default_security_class_name)
-{
-/**************************************
- *
- *      M E T _ r e l a t i o n _ d e f a u l t _ c l a s s
- *
- **************************************
- *
- * Functional description
- *      Checks that a given security class is the default for
- *      a given relation, returning TRUE if there's a match.
- *      It can be made obsolete in the future if jrd_rel struct
- *	gets another field, although metadata loading order
- *	would not be safe when compared with this function.
- *
- **************************************/
-	SET_TDBB (tdbb);
-	Database* dbb = tdbb->getDatabase();
-	CHECK_DBB (dbb);
-
-	bool found = false;
-	jrd_req* request = CMP_find_request (tdbb, irq_l_relation_defsec, IRQ_REQUESTS);
-
-	FOR (REQUEST_HANDLE request)
-		REL IN RDB$RELATIONS WITH REL.RDB$RELATION_NAME EQ relation_name.c_str()
-
-		if (!REQUEST (irq_l_relation_defsec))
-			REQUEST (irq_l_relation_defsec) = request;
-
-		if (!REL.RDB$DEFAULT_CLASS.NULL) {
-			if (default_security_class_name == REL.RDB$DEFAULT_CLASS)
-			{
-				found = true;
-			}
-		}
-
-	END_FOR;
-
-	if (!REQUEST (irq_l_relation_defsec))
-		REQUEST (irq_l_relation_defsec) = request;
-
-	return found;
-}
-
-
 void MET_release_existence(thread_db* tdbb, jrd_rel* relation)
 {
 /**************************************
--- a/src/jrd/met_proto.h
+++ b/src/jrd/met_proto.h
@ -85,7 +85,7 @@ void		MET_lookup_cnstrt_for_index(Jrd::thread_db*, Firebird::MetaName& constrain
 void		MET_lookup_cnstrt_for_trigger(Jrd::thread_db*, Firebird::MetaName&, Firebird::MetaName&, const Firebird::MetaName&);
 void		MET_lookup_exception(Jrd::thread_db*, SLONG, /* OUT */ Firebird::MetaName&, /* OUT */ TEXT*, size_t);
 SLONG		MET_lookup_exception_number(Jrd::thread_db*, const Firebird::MetaName&);
-int			MET_lookup_field(Jrd::thread_db*, Jrd::jrd_rel*, const Firebird::MetaName&, const Firebird::MetaName*);
+int			MET_lookup_field(Jrd::thread_db*, Jrd::jrd_rel*, const Firebird::MetaName&);
 Jrd::BlobFilter*	MET_lookup_filter(Jrd::thread_db*, SSHORT, SSHORT);
 SLONG		MET_lookup_generator(Jrd::thread_db*, const TEXT*);
 void		MET_lookup_generator_id(Jrd::thread_db*, SLONG, Firebird::MetaName&);
@ -103,7 +103,6 @@ void		MET_post_existence(Jrd::thread_db*, Jrd::jrd_rel*);
 void		MET_prepare(Jrd::thread_db*, Jrd::jrd_tra*, USHORT, const UCHAR*);
 Jrd::jrd_prc*	MET_procedure(Jrd::thread_db*, int, bool, USHORT);
 Jrd::jrd_rel*	MET_relation(Jrd::thread_db*, USHORT);
-bool		MET_relation_default_class (Jrd::thread_db*, const Firebird::MetaName&, const Firebird::MetaName&);
 void		MET_release_existence(Jrd::thread_db*, Jrd::jrd_rel*);
 void		MET_release_trigger(Jrd::thread_db*, Jrd::trig_vec**, const Firebird::MetaName&);
 void		MET_release_triggers(Jrd::thread_db*, Jrd::trig_vec**);
--- a/src/jrd/par.cpp
+++ b/src/jrd/par.cpp
@ -554,7 +554,7 @@ jrd_nod* PAR_make_field(thread_db* tdbb, CompilerScratch* csb,
 	jrd_prc* procedure = csb->csb_rpt[stream].csb_procedure;

 	const SSHORT id = procedure ? find_proc_field(procedure, base_field) :
-		MET_lookup_field (tdbb, csb->csb_rpt[stream].csb_relation, base_field, 0);
+		MET_lookup_field (tdbb, csb->csb_rpt[stream].csb_relation, base_field);

 	if (id < 0)
 		return NULL;
@ -1337,7 +1337,7 @@ static jrd_nod* par_field(thread_db* tdbb, CompilerScratch* csb, SSHORT blr_oper
 			}

 			par_name(csb, name);
-			if ((id = MET_lookup_field(tdbb, relation, name.c_str(), 0)) < 0)
+			if ((id = MET_lookup_field(tdbb, relation, name)) < 0)
 			{
 				if (csb->csb_g_flags & csb_validation) {
 					id = 0;
--- a/src/jrd/scl.epp
+++ b/src/jrd/scl.epp
@ -112,7 +112,8 @@ void SCL_check_access(thread_db* tdbb,
 					  const Firebird::MetaName& prc_name,
 					  SecurityClass::flags_t mask,
 					  const TEXT* type,
-					  const char* name)
+					  const Firebird::MetaName& name,
+					  const Firebird::MetaName& r_name)
 {
 /**************************************
 *
@ -184,55 +185,24 @@ void SCL_check_access(thread_db* tdbb,
 		}
 	}

-	if (denied_db) {
+	if (denied_db)
+	{
 		ERR_post(Arg::Gds(isc_no_priv) << Arg::Str(names->p_names_string) <<
-										  Arg::Str("DATABASE") <<
+										  Arg::Str(object_database) <<
 										  Arg::Str(""));
 	}
-	else {
+	else
+	{
+		const Firebird::string full_name = r_name.hasData() ?
+			r_name.c_str() + Firebird::string(".") + name.c_str() : name.c_str();
+
 		ERR_post(Arg::Gds(isc_no_priv) << Arg::Str(names->p_names_string) <<
 										  Arg::Str(type) <<
-										  Arg::Str(name));
+										  Arg::Str(full_name));
 	}
 }


-void SCL_check_access(thread_db* tdbb,
-					  const SecurityClass* s_class,
-					  SLONG view_id,
-					  const Firebird::MetaName& trg_name,
-					  const Firebird::MetaName& prc_name,
-					  SecurityClass::flags_t mask,
-					  const TEXT* type,
-					  const Firebird::MetaName& name,
-					  const Firebird::MetaName& r_name)
-{
-/**************************************
- *
- *	S C L _ c h e c k _ a c c e s s
- *
- **************************************
- *
- * Functional description
- *	Check security class for desired permission.
- *	Alternate entrypoint.
- *
- **************************************/
-
-	SET_TDBB(tdbb);
-
-	Firebird::string fullFieldName(name.c_str());
-	if (r_name.hasData())
-	{
-		fullFieldName = r_name.c_str();
-		fullFieldName += '.';
-		fullFieldName += name.c_str();
-	}
-
-	SCL_check_access(tdbb, s_class, view_id, trg_name, prc_name, mask, type, fullFieldName.c_str());
-}
-
-
 void SCL_check_index(thread_db* tdbb, const Firebird::MetaName& index_name, UCHAR index_id,
 	SecurityClass::flags_t mask)
 {
@ -337,17 +307,10 @@ void SCL_check_index(thread_db* tdbb, const Firebird::MetaName& index_name, UCHA
 			WITH RF.RDB$RELATION_NAME EQ reln_name.c_str()
 			AND ISEG.RDB$INDEX_NAME EQ idx_name_ptr->c_str()

-		Firebird::string fullFieldName(reln_name.c_str());
-		fullFieldName += '.';
-		fullFieldName += RF.RDB$FIELD_NAME;
-		fullFieldName.rtrim();
-        if (!RF.RDB$SECURITY_CLASS.NULL) {
-			s_class = SCL_get_class(tdbb, RF.RDB$SECURITY_CLASS);
-			SCL_check_access(tdbb, s_class, 0, NULL, NULL, mask, object_column, fullFieldName);
-		}
-		else {
-			SCL_check_access(tdbb, default_s_class, 0, NULL, NULL, mask, object_column, fullFieldName);
-		}
+		s_class = (!RF.RDB$SECURITY_CLASS.NULL) ?
+			SCL_get_class(tdbb, RF.RDB$SECURITY_CLASS) : default_s_class;
+		SCL_check_access(tdbb, s_class, 0, NULL, NULL, mask,
+						 object_column, RF.RDB$FIELD_NAME, reln_name);

 	END_FOR;

@ -547,7 +510,7 @@ SecurityClass::flags_t SCL_get_mask(thread_db* tdbb, const TEXT* relation_name,
 		const jrd_fld* field;
 		SSHORT id;
 		if (field_name &&
-			(id = MET_lookup_field(tdbb, relation, field_name, 0)) >= 0 &&
+			(id = MET_lookup_field(tdbb, relation, field_name)) >= 0 &&
 			(field = MET_get_field(relation, id)) &&
 			(s_class = SCL_get_class(tdbb, field->fld_security_name.c_str())))
 		{
--- a/src/jrd/scl.h
+++ b/src/jrd/scl.h
@ -147,6 +147,7 @@ public:
 	~UserId();
 };

+const char* const object_database	= "DATABASE";
 const char* const object_table		= "TABLE";
 const char* const object_procedure	= "PROCEDURE";
 const char* const object_column		= "COLUMN";
--- a/src/jrd/scl_proto.h
+++ b/src/jrd/scl_proto.h
@ -27,40 +27,11 @@
 #include "../jrd/scl.h"
 #include "../common/classes/array.h"

-//namespace Jrd {
-//	class SecurityClass;
-//}
-
 struct dsc;

-void SCL_check_access(Jrd::thread_db*, const Jrd::SecurityClass*, SLONG, const Firebird::MetaName&,
-					  const Firebird::MetaName&, Jrd::SecurityClass::flags_t, const TEXT*, const char*);
 void SCL_check_access(Jrd::thread_db*, const Jrd::SecurityClass*, SLONG, const Firebird::MetaName&,
 					  const Firebird::MetaName&, Jrd::SecurityClass::flags_t,
-					  const TEXT*, const Firebird::MetaName&, const Firebird::MetaName&);
-inline void SCL_check_access(Jrd::thread_db* tdbb,
-							 const Jrd::SecurityClass* s_class,
-							 SLONG view_id,
-							 const Firebird::MetaName& trg_name,
-							 const Firebird::MetaName& prc_name,
-							 Jrd::SecurityClass::flags_t mask,
-							 const TEXT* type,
-							 const Firebird::string& name)
-{
-	SCL_check_access(tdbb, s_class, view_id, trg_name, prc_name, mask, type, name.c_str());
-}
-inline void SCL_check_access(Jrd::thread_db* tdbb,
-							 const Jrd::SecurityClass* s_class,
-							 SLONG view_id,
-							 const Firebird::MetaName& trg_name,
-							 const Firebird::MetaName& prc_name,
-							 Jrd::SecurityClass::flags_t mask,
-							 const TEXT* type,
-							 const Firebird::MetaName& name)
-{
-	SCL_check_access(tdbb, s_class, view_id, trg_name, prc_name, mask, type, name.c_str());
-}
-
+					  const TEXT*, const Firebird::MetaName&, const Firebird::MetaName& = "");
 void SCL_check_index(Jrd::thread_db*, const Firebird::MetaName&, UCHAR, Jrd::SecurityClass::flags_t);
 void SCL_check_procedure(Jrd::thread_db* tdbb, const dsc*, Jrd::SecurityClass::flags_t);
 void SCL_check_relation(Jrd::thread_db* tdbb, const dsc*, Jrd::SecurityClass::flags_t);
--- a/src/jrd/vio.cpp
+++ b/src/jrd/vio.cpp
@ -3409,7 +3409,7 @@ static void check_class(thread_db* tdbb,
 	Attachment* attachment = tdbb->getAttachment();

 	SCL_check_access(tdbb, attachment->att_security_class, 0, NULL, NULL, SCL_protect,
-					 "DATABASE", NULL);
+					 object_database, "");
 	DFW_post_work(transaction, dfw_compute_security, &desc2, 0);
 }

@ -3432,7 +3432,7 @@ static void check_control(thread_db* tdbb)
 	Attachment* attachment = tdbb->getAttachment();

 	SCL_check_access(tdbb, attachment->att_security_class, 0, NULL, NULL, SCL_control,
-					 "DATABASE", NULL);
+					 object_database, "");
 }