Skip to content

Commit

Permalink
Fix #7931 - Incorrect variable usage using UPDATE OR INSERT.
Browse files Browse the repository at this point in the history
  • Loading branch information
asfernandes committed Dec 20, 2023
1 parent 78ff27d commit 305c40a
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 15 deletions.
2 changes: 2 additions & 0 deletions src/dsql/DdlNodes.epp
Original file line number Diff line number Diff line change
Expand Up @@ -1661,6 +1661,7 @@ string CreateAlterFunctionNode::internalPrint(NodePrinter& printer) const
DdlNode* CreateAlterFunctionNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
{
dsqlScratch->flags |= (DsqlCompilerScratch::FLAG_BLOCK | DsqlCompilerScratch::FLAG_FUNCTION);
dsqlScratch->reserveInitialVarNumbers(1);

LocalDeclarationsNode::checkUniqueFieldsNames(localDeclList, &parameters, nullptr);

Expand Down Expand Up @@ -2677,6 +2678,7 @@ string CreateAlterProcedureNode::internalPrint(NodePrinter& printer) const
DdlNode* CreateAlterProcedureNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
{
dsqlScratch->flags |= (DsqlCompilerScratch::FLAG_BLOCK | DsqlCompilerScratch::FLAG_PROCEDURE);
dsqlScratch->reserveInitialVarNumbers(returns.getCount());

LocalDeclarationsNode::checkUniqueFieldsNames(localDeclList, &parameters, &returns);

Expand Down
6 changes: 2 additions & 4 deletions src/dsql/DsqlCompilerScratch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,6 @@ void DsqlCompilerScratch::putLocalVariableDecl(dsql_var* variable, DeclareVariab
if (variable->field->fld_name.hasData()) // Not a function return value
putDebugVariable(variable->number, variable->field->fld_name);

++hiddenVarsNumber;

if (variable->type != dsql_var::TYPE_INPUT && hostParam && hostParam->dsqlDef->defaultClause)
{
hostParam->dsqlDef->defaultClause->value =
Expand Down Expand Up @@ -377,7 +375,7 @@ void DsqlCompilerScratch::putOuterMaps()

// Make a variable.
dsql_var* DsqlCompilerScratch::makeVariable(dsql_fld* field, const char* name,
const dsql_var::Type type, USHORT msgNumber, USHORT itemNumber, USHORT localNumber)
const dsql_var::Type type, USHORT msgNumber, USHORT itemNumber, std::optional<USHORT> localNumber)
{
DEV_BLKCHK(field, dsql_type_fld);

Expand All @@ -387,7 +385,7 @@ dsql_var* DsqlCompilerScratch::makeVariable(dsql_fld* field, const char* name,
dsqlVar->type = type;
dsqlVar->msgNumber = msgNumber;
dsqlVar->msgItem = itemNumber;
dsqlVar->number = localNumber;
dsqlVar->number = localNumber.has_value() ? localNumber.value() : nextVarNumber++;
dsqlVar->field = field;

if (field)
Expand Down
19 changes: 15 additions & 4 deletions src/dsql/DsqlCompilerScratch.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "../jrd/MetaName.h"
#include "../common/classes/stack.h"
#include "../common/classes/alloc.h"
#include <optional>

namespace Jrd
{
Expand Down Expand Up @@ -164,7 +165,7 @@ class DsqlCompilerScratch : public BlrDebugWriter

void putOuterMaps();
dsql_var* makeVariable(dsql_fld*, const char*, const dsql_var::Type type, USHORT,
USHORT, USHORT);
USHORT, std::optional<USHORT> = std::nullopt);
dsql_var* resolveVariable(const MetaName& varName);
void genReturn(bool eosFlag = false);

Expand All @@ -178,8 +179,7 @@ class DsqlCompilerScratch : public BlrDebugWriter
context->clear();
contextNumber = 0;
derivedContextNumber = 0;

hiddenVarsNumber = 0;
nextVarNumber = 0;
hiddenVariables.clear();
}

Expand Down Expand Up @@ -234,6 +234,17 @@ class DsqlCompilerScratch : public BlrDebugWriter
}
}

USHORT reserveVarNumber()
{
return nextVarNumber++;
}

void reserveInitialVarNumbers(USHORT count)
{
fb_assert(nextVarNumber == 0);
nextVarNumber = count;
}

bool isPsql() const { return psql; }
void setPsql(bool value) { psql = value; }

Expand Down Expand Up @@ -298,7 +309,6 @@ class DsqlCompilerScratch : public BlrDebugWriter
bool processingWindow = false; // processing window functions
bool checkConstraintTrigger = false; // compiling a check constraint trigger
dsc domainValue; // VALUE in the context of domain's check constraint
USHORT hiddenVarsNumber = 0; // next hidden variable number
Firebird::Array<dsql_var*> hiddenVariables; // hidden variables
Firebird::Array<dsql_var*> variables;
Firebird::Array<dsql_var*> outputVariables;
Expand All @@ -311,6 +321,7 @@ class DsqlCompilerScratch : public BlrDebugWriter
private:
Firebird::HalfStaticArray<SelectExprNode*, 4> ctes; // common table expressions
Firebird::HalfStaticArray<const Firebird::string*, 4> cteAliases; // CTE aliases in recursive members
USHORT nextVarNumber = 0; // Next available variable number
bool psql = false;
Firebird::LeftPooledMap<MetaName, DeclareSubFuncNode*> subFunctions;
Firebird::LeftPooledMap<MetaName, DeclareSubProcNode*> subProcedures;
Expand Down
2 changes: 1 addition & 1 deletion src/dsql/ExprNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13991,7 +13991,7 @@ ValueExprNode* VariableNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
else
{
if (!dsqlScratch->outerVarsMap.exist(node->dsqlVar->number))
dsqlScratch->outerVarsMap.put(node->dsqlVar->number, dsqlScratch->hiddenVarsNumber++);
dsqlScratch->outerVarsMap.put(node->dsqlVar->number, dsqlScratch->reserveVarNumber());
}
}
}
Expand Down
9 changes: 3 additions & 6 deletions src/dsql/StmtNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4950,6 +4950,7 @@ ExecBlockNode* ExecBlockNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
statement->setType(DsqlStatement::TYPE_EXEC_BLOCK);

dsqlScratch->flags |= DsqlCompilerScratch::FLAG_BLOCK;
dsqlScratch->reserveInitialVarNumbers(parameters.getCount() + returns.getCount());

ExecBlockNode* node = FB_NEW_POOL(dsqlScratch->getPool()) ExecBlockNode(dsqlScratch->getPool());

Expand Down Expand Up @@ -6128,7 +6129,6 @@ void LocalDeclarationsNode::genBlr(DsqlCompilerScratch* dsqlScratch)
// Sub routine doesn't need ports and should generate BLR as declared in its metadata.
const bool isSubRoutine = dsqlScratch->flags & DsqlCompilerScratch::FLAG_SUB_ROUTINE;
const auto& variables = isSubRoutine ? dsqlScratch->outputVariables : dsqlScratch->variables;
USHORT locals = variables.getCount();

Array<dsql_var*> declaredVariables;

Expand Down Expand Up @@ -6160,16 +6160,14 @@ void LocalDeclarationsNode::genBlr(DsqlCompilerScratch* dsqlScratch)
}

const auto variable = dsqlScratch->makeVariable(field, field->fld_name.c_str(),
dsql_var::TYPE_LOCAL, 0, 0, locals);
dsql_var::TYPE_LOCAL, 0, 0);
declaredVariables.add(variable);

dsqlScratch->putLocalVariableDecl(variable, varNode, varNode->dsqlDef->type->collate);

// Some field attributes are calculated inside putLocalVariable(), so we reinitialize
// the descriptor.
DsqlDescMaker::fromField(&variable->desc, field);

++locals;
}
else if (nodeIs<DeclareCursorNode>(parameter) ||
nodeIs<DeclareSubProcNode>(parameter) ||
Expand Down Expand Up @@ -10982,8 +10980,7 @@ static VariableNode* dsqlPassHiddenVariable(DsqlCompilerScratch* dsqlScratch, Va
}

VariableNode* varNode = FB_NEW_POOL(*tdbb->getDefaultPool()) VariableNode(*tdbb->getDefaultPool());
varNode->dsqlVar = dsqlScratch->makeVariable(NULL, "", dsql_var::TYPE_HIDDEN,
0, 0, dsqlScratch->hiddenVarsNumber++);
varNode->dsqlVar = dsqlScratch->makeVariable(nullptr, "", dsql_var::TYPE_HIDDEN, 0, 0);

DsqlDescMaker::fromNode(dsqlScratch, &varNode->dsqlVar->desc, expr);
varNode->setDsqlDesc(varNode->dsqlVar->desc);
Expand Down

0 comments on commit 305c40a

Please sign in to comment.