Fixed bug CORE-6351 : Computed field could be wrongly evaluated as NULL

src/dsql/ExprNodes.cpp

@@ -5619,7 +5619,12 @@ ValueExprNode* FieldNode::pass1(thread_db* tdbb, CompilerScratch* csb)
 	jrd_fld* field;
 
 	if (!relation || !(field = MET_get_field(relation, fieldId)))
+	{
+		if (relation && (relation->rel_flags & REL_being_scanned))
+			csb->csb_g_flags |= csb_reload;
+
 		return ValueExprNode::pass1(tdbb, csb);
+	}
 
 	dsc desc;
 	getDesc(tdbb, csb, &desc);
@@ -5863,6 +5868,15 @@ dsc* FieldNode::execute(thread_db* tdbb, jrd_req* request) const
 	Record* record = rpb.rpb_record;
 	jrd_rel* relation = rpb.rpb_relation;
 
+#ifdef DEV_BUILD
+	if (relation && !relation->isView())
+	{
+		// Computed fields shouldn't be present at this point
+		jrd_fld* field = MET_get_field(relation, fieldId);
+		fb_assert(field && !field->fld_computation);
+	}
+#endif
+
 	// In order to "map a null to a default" value (in EVL_field()), the relation block is referenced.
 	// Reference: Bug 10116, 10424
 
@@ -11307,6 +11321,8 @@ dsc* UdfCallNode::execute(thread_db* tdbb, jrd_req* request) const
 	}
 	else
 	{
+		const_cast<Function*>(function.getObject())->checkReload(tdbb);
+
 		Jrd::Attachment* attachment = tdbb->getAttachment();
 
 		const ULONG inMsgLength = function->getInputFormat() ? function->getInputFormat()->fmt_length : 0;

src/dsql/StmtNodes.cpp

@@ -2937,6 +2937,8 @@ void ExecProcedureNode::executeProcedure(thread_db* tdbb, jrd_req* request) cons
 				Arg::Str(procedure->getName().identifier) << Arg::Str(procedure->getName().package));
 	}
 
+	const_cast<jrd_prc*>(procedure.getObject())->checkReload(tdbb);
+
 	Jrd::Attachment* attachment = tdbb->getAttachment();
 
 	ULONG inMsgLength = 0;

src/jrd/Function.epp

@@ -424,12 +424,10 @@ Function* Function::loadMetadata(thread_db* tdbb, USHORT id, bool noscan, USHORT
 					}
 					else if (!X.RDB$FUNCTION_BLR.NULL)
 					{
-						if (!X.RDB$DEBUG_INFO.NULL)
-							DBG_parse_debug_info(tdbb, &X.RDB$DEBUG_INFO, *csb->csb_dbg_info);
-
 						try
 						{
-							function->parseBlr(tdbb, csb, &X.RDB$FUNCTION_BLR);
+							function->parseBlr(tdbb, csb, &X.RDB$FUNCTION_BLR,
+								X.RDB$DEBUG_INFO.NULL ? NULL : &X.RDB$DEBUG_INFO);
 						}
 						catch (const Exception& ex)
 						{
@@ -539,3 +537,47 @@ void Function::clearCache(thread_db* tdbb)
 {
 	tdbb->getAttachment()->att_functions[getId()] = NULL;
 }
+
+bool Function::reload(thread_db* tdbb)
+{
+	fb_assert(this->flags & Routine::FLAG_RELOAD);
+
+	Attachment* attachment = tdbb->getAttachment();
+	AutoCacheRequest request(tdbb, irq_l_funct_blr, IRQ_REQUESTS);
+
+	FOR(REQUEST_HANDLE request)
+		X IN RDB$FUNCTIONS
+		WITH X.RDB$FUNCTION_ID EQ this->getId()
+	{
+		if (X.RDB$FUNCTION_BLR.NULL)
+			continue;
+
+		MemoryPool* const csb_pool = attachment->createPool();
+		Jrd::ContextPoolHolder context(tdbb, csb_pool);
+
+		AutoPtr<CompilerScratch> csb(CompilerScratch::newCsb(*csb_pool, 5));
+
+		try
+		{
+			this->parseBlr(tdbb, csb, &X.RDB$FUNCTION_BLR,
+				X.RDB$DEBUG_INFO.NULL ? NULL : &X.RDB$DEBUG_INFO);
+
+			// parseBlr() above could set FLAG_RELOAD again
+			return !(this->flags & Routine::FLAG_RELOAD);
+		}
+		catch (const Exception& ex)
+		{
+			StaticStatusVector temp_status;
+			ex.stuffException(temp_status);
+
+			attachment->deletePool(csb_pool);
+
+			const string name = this->getName().toString();
+			(Arg::Gds(isc_bad_fun_BLR) << Arg::Str(name)
+				<< Arg::StatusVector(temp_status.begin())).raise();
+		}
+	}
+	END_FOR
+
+	return false;
+}

src/jrd/Function.h

@@ -80,6 +80,9 @@ namespace Jrd
 
 		bool fun_deterministic;
 		const ExtEngineManager::Function* fun_external;
+
+	protected:
+		virtual bool reload(thread_db* tdbb);
 	};
 }
 

src/jrd/Routine.cpp

@@ -107,11 +107,30 @@ Format* Routine::createFormat(MemoryPool& pool, IMessageMetadata* params, bool a
 	return format;
 }
 
-// Parse routine BLR.
-void Routine::parseBlr(thread_db* tdbb, CompilerScratch* csb, bid* blob_id)
+void Routine::checkReload(thread_db* tdbb)
+{
+	if (!(flags & FLAG_RELOAD))
+		return;
+
+	if (!reload(tdbb))
+	{
+		string err;
+		err.printf("Recompile of %s \"%s\" failed", 
+					getObjectType() == obj_udf ? "FUNCTION" : "PROCEDURE",
+					getName().toString().c_str());
+
+		(Arg::Gds(isc_random) << Arg::Str(err)).raise();
+	}
+}
+
+// Parse routine BLR and debug info.
+void Routine::parseBlr(thread_db* tdbb, CompilerScratch* csb, bid* blob_id, bid* blobDbg)
 {
 	Jrd::Attachment* attachment = tdbb->getAttachment();
 
+	if (blobDbg)
+		DBG_parse_debug_info(tdbb, blobDbg, *csb->csb_dbg_info);
+
 	UCharBuffer tmp;
 
 	if (blob_id)
@@ -125,10 +144,15 @@ void Routine::parseBlr(thread_db* tdbb, CompilerScratch* csb, bid* blob_id)
 
 	parseMessages(tdbb, csb, BlrReader(tmp.begin(), (unsigned) tmp.getCount()));
 
+	flags &= ~Routine::FLAG_RELOAD;
+
 	JrdStatement* statement = getStatement();
 	PAR_blr(tdbb, NULL, tmp.begin(), (ULONG) tmp.getCount(), NULL, &csb, &statement, false, 0);
 	setStatement(statement);
 
+	if (csb->csb_g_flags & csb_reload)
+		flags |= FLAG_RELOAD;
+
 	if (!blob_id)
 		setImplemented(false);
 }

src/jrd/Routine.h

@@ -78,6 +78,7 @@ namespace Jrd
 															// so dfw.epp:modify_procedure() can flip procedure body without
 															// invalidating procedure pointers from other parts of metadata cache
 		static const USHORT FLAG_CHECK_EXISTENCE	= 16;	// Existence lock released
+		static const USHORT FLAG_RELOAD		 		= 32;	// Recompile before execution
 
 		static const USHORT MAX_ALTER_COUNT = 64;	// Number of times an in-cache routine can be altered
 
@@ -112,6 +113,8 @@ namespace Jrd
 		bool isDefined() const { return defined; }
 		void setDefined(bool value) { defined = value; }
 
+		void checkReload(thread_db* tdbb);
+
 		USHORT getDefaultCount() const { return defaultCount; }
 		void setDefaultCount(USHORT value) { defaultCount = value; }
 
@@ -127,7 +130,7 @@ namespace Jrd
 		const Firebird::Array<NestConst<Parameter> >& getOutputFields() const { return outputFields; }
 		Firebird::Array<NestConst<Parameter> >& getOutputFields() { return outputFields; }
 
-		void parseBlr(thread_db* tdbb, CompilerScratch* csb, bid* blob_id);
+		void parseBlr(thread_db* tdbb, CompilerScratch* csb, bid* blob_id, bid* blobDbg);
 		void parseMessages(thread_db* tdbb, CompilerScratch* csb, Firebird::BlrReader blrReader);
 
 		bool isUsed() const
@@ -168,6 +171,10 @@ namespace Jrd
 		Firebird::Array<NestConst<Parameter> > inputFields;		// array of field blocks
 		Firebird::Array<NestConst<Parameter> > outputFields;	// array of field blocks
 
+	protected:
+
+		virtual bool reload(thread_db* tdbb) = 0;
+
 	public:
 		USHORT flags;
 		USHORT useCount;		// requests compiled with routine

src/jrd/exe.h

@@ -610,6 +610,7 @@ const int csb_post_trigger		= 32;	// this is an AFTER trigger
 const int csb_validation		= 64;	// we're in a validation expression (RDB hack)
 const int csb_reuse_context		= 128;	// allow context reusage
 const int csb_subroutine		= 256;	// sub routine
+const int csb_reload			= 512;	// request's BLR should be loaded and parsed again
 
 // CompilerScratch.csb_rpt[].csb_flags's values.
 const int csb_active		= 1;		// stream is active

src/jrd/irq.h

@@ -51,6 +51,7 @@ enum irq_type_t
 	irq_v_security_o,		// verify security for role
 	irq_l_index,			// lookup index id
 	irq_l_functions,		// lookup function
+	irq_l_funct_blr,		// lookup function BLR and debug info
 	irq_l_args,				// lookup function arguments
 	irq_s_triggers,			// scan triggers
 	irq_s_triggers2,		// scan triggers
@@ -90,6 +91,7 @@ enum irq_type_t
 	irq_r_params,			// scan procedure parameters
 
 	irq_r_procedure,		// scan procedure
+	irq_r_proc_blr,			// look for procedure's BLR and debug info
 	irq_pkg_security,		// verify security for package
 	irq_p_security,			// verify security for procedure
 	irq_c_prc_dpd,			// create procedure dependencies for delete

src/jrd/jrd.h

@@ -250,6 +250,9 @@ public:
 
 	virtual bool checkCache(thread_db* tdbb) const;
 	virtual void clearCache(thread_db* tdbb);
+
+protected:
+	virtual bool reload(thread_db* tdbb);	// impl is in met.epp
 };
 
 

src/jrd/met.epp

@@ -3472,12 +3472,10 @@ jrd_prc* MET_procedure(thread_db* tdbb, USHORT id, bool noscan, USHORT flags)
 					}
 					else
 					{
-						if (!P.RDB$DEBUG_INFO.NULL)
-							DBG_parse_debug_info(tdbb, &P.RDB$DEBUG_INFO, *csb->csb_dbg_info);
-
 						try
 						{
-							procedure->parseBlr(tdbb, csb, &P.RDB$PROCEDURE_BLR);
+							procedure->parseBlr(tdbb, csb, &P.RDB$PROCEDURE_BLR, 
+								P.RDB$DEBUG_INFO.NULL ? NULL : &P.RDB$DEBUG_INFO);
 						}
 						catch (const Exception& ex)
 						{
@@ -3563,6 +3561,45 @@ jrd_prc* MET_procedure(thread_db* tdbb, USHORT id, bool noscan, USHORT flags)
 	return procedure;
 }
 
+bool jrd_prc::reload(thread_db* tdbb)
+{
+	fb_assert(this->flags & Routine::FLAG_RELOAD);
+
+	Attachment* attachment = tdbb->getAttachment();
+	AutoCacheRequest request(tdbb, irq_r_proc_blr, IRQ_REQUESTS);
+
+	FOR(REQUEST_HANDLE request)
+		P IN RDB$PROCEDURES WITH P.RDB$PROCEDURE_ID EQ this->getId()
+	{
+		MemoryPool* const csb_pool = attachment->createPool();
+		Jrd::ContextPoolHolder context(tdbb, csb_pool);
+
+		AutoPtr<CompilerScratch> csb(CompilerScratch::newCsb(*csb_pool, 5));
+
+		try
+		{
+			this->parseBlr(tdbb, csb, &P.RDB$PROCEDURE_BLR,
+				P.RDB$DEBUG_INFO.NULL ? NULL : &P.RDB$DEBUG_INFO);
+
+			// parseBlr() above could set FLAG_RELOAD again
+			return !(this->flags & Routine::FLAG_RELOAD);
+		}
+		catch (const Exception& ex)
+		{
+			StaticStatusVector temp_status;
+			ex.stuffException(temp_status);
+
+			attachment->deletePool(csb_pool);
+
+			const string name = this->getName().toString();
+			(Arg::Gds(isc_bad_proc_BLR) << Arg::Str(name)
+				<< Arg::StatusVector(temp_status.begin())).raise();
+		}
+	}
+	END_FOR
+
+	return false;
+}
 
 jrd_rel* MET_relation(thread_db* tdbb, USHORT id)
 {

src/jrd/recsrc/ProcedureScan.cpp

@@ -62,6 +62,8 @@ void ProcedureScan::open(thread_db* tdbb) const
 				Arg::Str(m_procedure->getName().identifier) << Arg::Str(m_procedure->getName().package));
 	}
 
+	const_cast<jrd_prc*>(m_procedure)->checkReload(tdbb);
+
 	jrd_req* const request = tdbb->getRequest();
 	Impure* const impure = request->getImpure<Impure>(m_impure);