Fixed #7651: Unable to find savepoint in insert with nested query and returning clause. Fixed also regression for #5613 regarding UPDATE OR INSERT.

Author: dyemanov

src/dsql/StmtNodes.cpp

@@ -7536,7 +7536,11 @@ StmtNode* StoreNode::internalDsqlPass(DsqlCompilerScratch* dsqlScratch,
 	else
 	{
 		values = doDsqlPass(dsqlScratch, dsqlValues, false);
-		needSavePoint = SubSelectFinder::find(dsqlScratch->getPool(), values);
+		// If this INSERT belongs to some PSQL code block and has subqueries
+		// inside its VALUES part, signal the caller to create a savepoint frame.
+		// See bug #5613 (aka CORE-5337) for details.
+		needSavePoint = (dsqlScratch->flags & DsqlCompilerScratch::FLAG_BLOCK) &&
+			SubSelectFinder::find(dsqlScratch->getPool(), values);
 	}
 
 	// Process relation
@@ -7681,13 +7685,9 @@ StmtNode* StoreNode::internalDsqlPass(DsqlCompilerScratch* dsqlScratch,
 StmtNode* StoreNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
 {
 	bool needSavePoint;
-	StmtNode* node = SavepointEncloseNode::make(dsqlScratch->getPool(), dsqlScratch,
-		internalDsqlPass(dsqlScratch, false, needSavePoint));
+	const auto node = internalDsqlPass(dsqlScratch, false, needSavePoint);
 
-	if (!needSavePoint || nodeIs<SavepointEncloseNode>(node))
-		return node;
-
-	return FB_NEW_POOL(dsqlScratch->getPool()) SavepointEncloseNode(dsqlScratch->getPool(), node);
+	return SavepointEncloseNode::make(dsqlScratch->getPool(), dsqlScratch, node, needSavePoint);
 }
 
 string StoreNode::internalPrint(NodePrinter& printer) const
@@ -8758,12 +8758,18 @@ DmlNode* SavepointEncloseNode::parse(thread_db* tdbb, MemoryPool& pool, Compiler
 	return node;
 }
 
-StmtNode* SavepointEncloseNode::make(MemoryPool& pool, DsqlCompilerScratch* dsqlScratch, StmtNode* node)
+StmtNode* SavepointEncloseNode::make(MemoryPool& pool, DsqlCompilerScratch* dsqlScratch, StmtNode* node, bool force)
 {
-	// Add savepoint wrapper around the statement having error handlers
+	// Add savepoint wrapper around the statement having error handlers, or if requested explicitly
 
-	return dsqlScratch->errorHandlers ?
-		FB_NEW_POOL(pool) SavepointEncloseNode(pool, node) : node;
+	if (dsqlScratch->errorHandlers || force)
+	{
+		// Ensure that savepoints are never created around a DSQL statement
+		fb_assert(dsqlScratch->flags & DsqlCompilerScratch::FLAG_BLOCK);
+		return FB_NEW_POOL(pool) SavepointEncloseNode(pool, node);
+	}
+
+	return node;
 }
 
 SavepointEncloseNode* SavepointEncloseNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
@@ -9417,7 +9423,7 @@ StmtNode* UpdateOrInsertNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
 	if (!returning)
 		dsqlScratch->getDsqlStatement()->setType(DsqlStatement::TYPE_INSERT);
 
-	return SavepointEncloseNode::make(dsqlScratch->getPool(), dsqlScratch, node);
+	return SavepointEncloseNode::make(dsqlScratch->getPool(), dsqlScratch, node, needSavePoint);
 }
 
 string UpdateOrInsertNode::internalPrint(NodePrinter& printer) const

src/dsql/StmtNodes.h

@@ -1444,17 +1444,10 @@ class ReturnNode final : public TypedNode<DsqlOnlyStmtNode, StmtNode::TYPE_RETUR
 
 class SavepointEncloseNode final : public TypedNode<StmtNode, StmtNode::TYPE_SAVEPOINT>
 {
-public:
-	explicit SavepointEncloseNode(MemoryPool& pool, StmtNode* stmt)
-		: TypedNode<StmtNode, StmtNode::TYPE_SAVEPOINT>(pool),
-		  statement(stmt)
-	{
-	}
-
 public:
 	static DmlNode* parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, const UCHAR blrOp);
 
-	static StmtNode* make(MemoryPool& pool, DsqlCompilerScratch* dsqlScratch, StmtNode* node);
+	static StmtNode* make(MemoryPool& pool, DsqlCompilerScratch* dsqlScratch, StmtNode* node, bool force = false);
 
 	virtual Firebird::string internalPrint(NodePrinter& printer) const;
 	virtual SavepointEncloseNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
@@ -1465,7 +1458,13 @@ class SavepointEncloseNode final : public TypedNode<StmtNode, StmtNode::TYPE_SAV
 
 	virtual const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* exeState) const;
 
-public:
+private:
+	explicit SavepointEncloseNode(MemoryPool& pool, StmtNode* stmt)
+		: TypedNode<StmtNode, StmtNode::TYPE_SAVEPOINT>(pool),
+		  statement(stmt)
+	{
+	}
+
 	NestConst<StmtNode> statement;
 };