Skip to content

Commit 9f834ab

Browse files
authored
Fixed CORE-5611 and CORE-5646 - pull request #114
- CORE-5611 - Higher memory consumption for prepared statements in FB3. - CORE-5646 - Parse error when compiling a statement causes memory leak until attachment is disconnected.
2 parents 0ffd7b5 + 5f5a869 commit 9f834ab

35 files changed

Lines changed: 1365 additions & 898 deletions

src/dsql/AggNodes.cpp

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,12 @@ AggNode::AggNode(MemoryPool& pool, const AggInfo& aAggInfo, bool aDistinct, bool
5656
ValueExprNode* aArg)
5757
: TypedNode<ValueExprNode, ExprNode::TYPE_AGGREGATE>(pool),
5858
aggInfo(aAggInfo),
59-
distinct(aDistinct),
60-
dialect1(aDialect1),
6159
arg(aArg),
6260
asb(NULL),
61+
distinct(aDistinct),
62+
dialect1(aDialect1),
6363
indexed(false)
6464
{
65-
addChildNode(arg, arg);
6665
}
6766

6867
DmlNode* AggNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR /*blrOp*/)
@@ -86,7 +85,10 @@ DmlNode* AggNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb,
8685

8786
const UCHAR count = csb->csb_blr_reader.getByte();
8887

89-
if (count != node->jrdChildNodes.getCount())
88+
NodeRefsHolder holder(pool);
89+
node->getChildren(holder, false);
90+
91+
if (count != holder.refs.getCount())
9092
PAR_error(csb, Arg::Gds(isc_funmismat) << name);
9193

9294
node->parseArgs(tdbb, csb, count);
@@ -146,7 +148,10 @@ bool AggNode::dsqlAggregateFinder(AggregateFinder& visitor)
146148
AutoSetRestore<USHORT> autoDeepestLevel(&visitor.deepestLevel, 0);
147149
AutoSetRestore<bool> autoIgnoreSubSelects(&visitor.ignoreSubSelects, true);
148150

149-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
151+
NodeRefsHolder holder(visitor.getPool());
152+
getChildren(holder, true);
153+
154+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
150155
visitor.visit((*i)->getExpr());
151156

152157
localDeepestLevel = visitor.deepestLevel;
@@ -175,7 +180,10 @@ bool AggNode::dsqlAggregateFinder(AggregateFinder& visitor)
175180

176181
AutoSetRestore<USHORT> autoDeepestLevel(&visitor.deepestLevel, localDeepestLevel);
177182

178-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
183+
NodeRefsHolder holder(visitor.getPool());
184+
getChildren(holder, true);
185+
186+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
179187
aggregate |= visitor.visit((*i)->getExpr());
180188
}
181189

@@ -188,9 +196,12 @@ bool AggNode::dsqlAggregate2Finder(Aggregate2Finder& visitor)
188196
return false;
189197

190198
bool found = false;
191-
FieldFinder fieldFinder(visitor.checkScopeLevel, visitor.matchType);
199+
FieldFinder fieldFinder(visitor.getPool(), visitor.checkScopeLevel, visitor.matchType);
200+
201+
NodeRefsHolder holder(visitor.getPool());
202+
getChildren(holder, true);
192203

193-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
204+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
194205
found |= fieldFinder.visit((*i)->getExpr());
195206

196207
if (!fieldFinder.getField())
@@ -234,13 +245,16 @@ bool AggNode::dsqlInvalidReferenceFinder(InvalidReferenceFinder& visitor)
234245

235246
if (!visitor.insideHigherMap)
236247
{
237-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
248+
NodeRefsHolder holder(visitor.dsqlScratch->getPool());
249+
getChildren(holder, true);
250+
251+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
238252
{
239253
// If there's another aggregate with the same scope_level or
240254
// an higher one then it's a invalid aggregate, because
241255
// aggregate-functions from the same context can't
242256
// be part of each other.
243-
if (Aggregate2Finder::find(visitor.context->ctx_scope_level,
257+
if (Aggregate2Finder::find(visitor.dsqlScratch->getPool(), visitor.context->ctx_scope_level,
244258
FIELD_MATCH_TYPE_EQUAL, false, (*i)->getExpr()))
245259
{
246260
// Nested aggregate functions are not allowed
@@ -260,7 +274,7 @@ bool AggNode::dsqlSubSelectFinder(SubSelectFinder& /*visitor*/)
260274

261275
ValueExprNode* AggNode::dsqlFieldRemapper(FieldRemapper& visitor)
262276
{
263-
AggregateFinder aggFinder(visitor.dsqlScratch, false);
277+
AggregateFinder aggFinder(visitor.getPool(), visitor.dsqlScratch, false);
264278
aggFinder.deepestLevel = visitor.dsqlScratch->scopeLevel;
265279
aggFinder.currentLevel = visitor.currentLevel;
266280

@@ -270,15 +284,18 @@ ValueExprNode* AggNode::dsqlFieldRemapper(FieldRemapper& visitor)
270284
return PASS1_post_map(visitor.dsqlScratch, this, visitor.context, visitor.windowNode);
271285
}
272286

273-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
287+
NodeRefsHolder holder(visitor.getPool());
288+
getChildren(holder, true);
289+
290+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
274291
(*i)->remap(visitor);
275292

276293
return this;
277294
}
278295

279-
bool AggNode::dsqlMatch(const ExprNode* other, bool ignoreMapCast) const
296+
bool AggNode::dsqlMatch(DsqlCompilerScratch* dsqlScratch, const ExprNode* other, bool ignoreMapCast) const
280297
{
281-
if (!ExprNode::dsqlMatch(other, ignoreMapCast))
298+
if (!ExprNode::dsqlMatch(dsqlScratch, other, ignoreMapCast))
282299
return false;
283300

284301
const AggNode* o = nodeAs<AggNode>(other);
@@ -297,6 +314,9 @@ void AggNode::setParameterName(dsql_par* parameter) const
297314

298315
void AggNode::genBlr(DsqlCompilerScratch* dsqlScratch)
299316
{
317+
NodeRefsHolder holder(dsqlScratch->getPool());
318+
getChildren(holder, true);
319+
300320
if (aggInfo.blr) // Is this a standard aggregate function?
301321
dsqlScratch->appendUChar((distinct ? aggInfo.distinctBlr : aggInfo.blr));
302322
else // This is a new window function.
@@ -306,7 +326,7 @@ void AggNode::genBlr(DsqlCompilerScratch* dsqlScratch)
306326

307327
unsigned count = 0;
308328

309-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
329+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
310330
{
311331
if (**i)
312332
++count;
@@ -315,7 +335,7 @@ void AggNode::genBlr(DsqlCompilerScratch* dsqlScratch)
315335
dsqlScratch->appendUChar(UCHAR(count));
316336
}
317337

318-
for (NodeRef** i = dsqlChildNodes.begin(); i != dsqlChildNodes.end(); ++i)
338+
for (NodeRef** i = holder.refs.begin(); i != holder.refs.end(); ++i)
319339
{
320340
if (**i)
321341
GEN_expr(dsqlScratch, (*i)->getExpr());
@@ -474,7 +494,6 @@ AvgAggNode::AvgAggNode(MemoryPool& pool, bool aDistinct, bool aDialect1, ValueEx
474494
: AggNode(pool, avgAggInfo, aDistinct, aDialect1, aArg),
475495
tempImpure(0)
476496
{
477-
dsqlCompatDialectVerb = "avg";
478497
}
479498

480499
DmlNode* AvgAggNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR blrOp)
@@ -703,7 +722,7 @@ dsc* AvgAggNode::aggExecute(thread_db* tdbb, jrd_req* request) const
703722

704723
AggNode* AvgAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
705724
{
706-
return FB_NEW_POOL(getPool()) AvgAggNode(getPool(), distinct, dialect1,
725+
return FB_NEW_POOL(dsqlScratch->getPool()) AvgAggNode(dsqlScratch->getPool(), distinct, dialect1,
707726
doDsqlPass(dsqlScratch, arg));
708727
}
709728

@@ -718,7 +737,6 @@ ListAggNode::ListAggNode(MemoryPool& pool, bool aDistinct, ValueExprNode* aArg,
718737
: AggNode(pool, listAggInfo, aDistinct, false, aArg),
719738
delimiter(aDelimiter)
720739
{
721-
addChildNode(delimiter, delimiter);
722740
}
723741

724742
DmlNode* ListAggNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR blrOp)
@@ -841,7 +859,7 @@ AggNode* ListAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
841859
{
842860
thread_db* tdbb = JRD_get_thread_data();
843861

844-
AggNode* node = FB_NEW_POOL(getPool()) ListAggNode(getPool(), distinct,
862+
AggNode* node = FB_NEW_POOL(dsqlScratch->getPool()) ListAggNode(dsqlScratch->getPool(), distinct,
845863
doDsqlPass(dsqlScratch, arg), doDsqlPass(dsqlScratch, delimiter));
846864

847865
dsc argDesc;
@@ -951,7 +969,7 @@ dsc* CountAggNode::aggExecute(thread_db* /*tdbb*/, jrd_req* request) const
951969

952970
AggNode* CountAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
953971
{
954-
return FB_NEW_POOL(getPool()) CountAggNode(getPool(), distinct, dialect1,
972+
return FB_NEW_POOL(dsqlScratch->getPool()) CountAggNode(dsqlScratch->getPool(), distinct, dialect1,
955973
doDsqlPass(dsqlScratch, arg));
956974
}
957975

@@ -964,7 +982,6 @@ static AggNode::Register<SumAggNode> sumAggInfo("SUM", blr_agg_total, blr_agg_to
964982
SumAggNode::SumAggNode(MemoryPool& pool, bool aDistinct, bool aDialect1, ValueExprNode* aArg)
965983
: AggNode(pool, sumAggInfo, aDistinct, aDialect1, aArg)
966984
{
967-
dsqlCompatDialectVerb = "sum";
968985
}
969986

970987
DmlNode* SumAggNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR blrOp)
@@ -1203,7 +1220,7 @@ dsc* SumAggNode::aggExecute(thread_db* /*tdbb*/, jrd_req* request) const
12031220

12041221
AggNode* SumAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
12051222
{
1206-
return FB_NEW_POOL(getPool()) SumAggNode(getPool(), distinct, dialect1,
1223+
return FB_NEW_POOL(dsqlScratch->getPool()) SumAggNode(dsqlScratch->getPool(), distinct, dialect1,
12071224
doDsqlPass(dsqlScratch, arg));
12081225
}
12091226

@@ -1294,7 +1311,8 @@ dsc* MaxMinAggNode::aggExecute(thread_db* /*tdbb*/, jrd_req* request) const
12941311

12951312
AggNode* MaxMinAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
12961313
{
1297-
return FB_NEW_POOL(getPool()) MaxMinAggNode(getPool(), type, doDsqlPass(dsqlScratch, arg));
1314+
return FB_NEW_POOL(dsqlScratch->getPool()) MaxMinAggNode(dsqlScratch->getPool(),
1315+
type, doDsqlPass(dsqlScratch, arg));
12981316
}
12991317

13001318

@@ -1503,7 +1521,8 @@ dsc* StdDevAggNode::aggExecute(thread_db* tdbb, jrd_req* request) const
15031521

15041522
AggNode* StdDevAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
15051523
{
1506-
return FB_NEW_POOL(getPool()) StdDevAggNode(getPool(), type, doDsqlPass(dsqlScratch, arg));
1524+
return FB_NEW_POOL(dsqlScratch->getPool()) StdDevAggNode(dsqlScratch->getPool(),
1525+
type, doDsqlPass(dsqlScratch, arg));
15071526
}
15081527

15091528

@@ -1527,7 +1546,6 @@ CorrAggNode::CorrAggNode(MemoryPool& pool, CorrType aType, ValueExprNode* aArg,
15271546
arg2(aArg2),
15281547
impure2Offset(0)
15291548
{
1530-
addChildNode(arg2, arg2);
15311549
}
15321550

15331551
void CorrAggNode::parseArgs(thread_db* tdbb, CompilerScratch* csb, unsigned /*count*/)
@@ -1765,7 +1783,7 @@ dsc* CorrAggNode::aggExecute(thread_db* tdbb, jrd_req* request) const
17651783

17661784
AggNode* CorrAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
17671785
{
1768-
return FB_NEW_POOL(getPool()) CorrAggNode(getPool(), type,
1786+
return FB_NEW_POOL(dsqlScratch->getPool()) CorrAggNode(dsqlScratch->getPool(), type,
17691787
doDsqlPass(dsqlScratch, arg), doDsqlPass(dsqlScratch, arg2));
17701788
}
17711789

@@ -1805,7 +1823,6 @@ RegrAggNode::RegrAggNode(MemoryPool& pool, RegrType aType, ValueExprNode* aArg,
18051823
arg2(aArg2),
18061824
impure2Offset(0)
18071825
{
1808-
addChildNode(arg2, arg2);
18091826
}
18101827

18111828
void RegrAggNode::parseArgs(thread_db* tdbb, CompilerScratch* csb, unsigned /*count*/)
@@ -2092,7 +2109,7 @@ dsc* RegrAggNode::aggExecute(thread_db* tdbb, jrd_req* request) const
20922109

20932110
AggNode* RegrAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
20942111
{
2095-
return FB_NEW_POOL(getPool()) RegrAggNode(getPool(), type,
2112+
return FB_NEW_POOL(dsqlScratch->getPool()) RegrAggNode(dsqlScratch->getPool(), type,
20962113
doDsqlPass(dsqlScratch, arg), doDsqlPass(dsqlScratch, arg2));
20972114
}
20982115

@@ -2106,7 +2123,6 @@ RegrCountAggNode::RegrCountAggNode(MemoryPool& pool, ValueExprNode* aArg, ValueE
21062123
: AggNode(pool, regrCountAggInfo, false, false, aArg),
21072124
arg2(aArg2)
21082125
{
2109-
addChildNode(arg2, arg2);
21102126
}
21112127

21122128
void RegrCountAggNode::parseArgs(thread_db* tdbb, CompilerScratch* csb, unsigned /*count*/)
@@ -2185,7 +2201,7 @@ dsc* RegrCountAggNode::aggExecute(thread_db* tdbb, jrd_req* request) const
21852201

21862202
AggNode* RegrCountAggNode::dsqlCopy(DsqlCompilerScratch* dsqlScratch) /*const*/
21872203
{
2188-
return FB_NEW_POOL(getPool()) RegrCountAggNode(getPool(),
2204+
return FB_NEW_POOL(dsqlScratch->getPool()) RegrCountAggNode(dsqlScratch->getPool(),
21892205
doDsqlPass(dsqlScratch, arg), doDsqlPass(dsqlScratch, arg2));
21902206
}
21912207

src/dsql/AggNodes.h

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ class AvgAggNode : public AggNode
3737

3838
static DmlNode* parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR blrOp);
3939

40+
virtual const char* getCompatDialectVerb()
41+
{
42+
return "avg";
43+
}
44+
4045
virtual unsigned getCapabilities() const
4146
{
4247
return CAP_RESPECTS_WINDOW_FRAME | CAP_WANTS_AGG_CALLS;
@@ -72,6 +77,12 @@ class ListAggNode : public AggNode
7277
return CAP_WANTS_AGG_CALLS;
7378
}
7479

80+
virtual void getChildren(NodeRefsHolder& holder, bool dsql) const
81+
{
82+
AggNode::getChildren(holder, dsql);
83+
holder.add(delimiter);
84+
}
85+
7586
virtual Firebird::string internalPrint(NodePrinter& printer) const;
7687
virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
7788
virtual bool setParameterType(DsqlCompilerScratch* dsqlScratch,
@@ -123,6 +134,11 @@ class SumAggNode : public AggNode
123134

124135
static DmlNode* parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR blrOp);
125136

137+
virtual const char* getCompatDialectVerb()
138+
{
139+
return "sum";
140+
}
141+
126142
virtual unsigned getCapabilities() const
127143
{
128144
return CAP_RESPECTS_WINDOW_FRAME | CAP_WANTS_AGG_CALLS;
@@ -259,6 +275,12 @@ class CorrAggNode : public AggNode
259275

260276
virtual void parseArgs(thread_db* tdbb, CompilerScratch* csb, unsigned count);
261277

278+
virtual void getChildren(NodeRefsHolder& holder, bool dsql) const
279+
{
280+
AggNode::getChildren(holder, dsql);
281+
holder.add(arg2);
282+
}
283+
262284
virtual Firebird::string internalPrint(NodePrinter& printer) const;
263285
virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
264286
virtual void getDesc(thread_db* tdbb, CompilerScratch* csb, dsc* desc);
@@ -318,6 +340,12 @@ class RegrAggNode : public AggNode
318340

319341
virtual void parseArgs(thread_db* tdbb, CompilerScratch* csb, unsigned count);
320342

343+
virtual void getChildren(NodeRefsHolder& holder, bool dsql) const
344+
{
345+
AggNode::getChildren(holder, dsql);
346+
holder.add(arg2);
347+
}
348+
321349
virtual Firebird::string internalPrint(NodePrinter& printer) const;
322350
virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
323351
virtual void getDesc(thread_db* tdbb, CompilerScratch* csb, dsc* desc);
@@ -353,6 +381,12 @@ class RegrCountAggNode : public AggNode
353381

354382
virtual void parseArgs(thread_db* tdbb, CompilerScratch* csb, unsigned count);
355383

384+
virtual void getChildren(NodeRefsHolder& holder, bool dsql) const
385+
{
386+
AggNode::getChildren(holder, dsql);
387+
holder.add(arg2);
388+
}
389+
356390
virtual Firebird::string internalPrint(NodePrinter& printer) const;
357391
virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
358392
virtual void getDesc(thread_db* tdbb, CompilerScratch* csb, dsc* desc);

0 commit comments

Comments
 (0)