Skip to content

Commit 283dc40

Browse files
AlexPeshkoffAlexPeshkoff
committed
Fixed an attempt to simultaneously modify table and create an index on it
1 parent cc171a1 commit 283dc40

File tree

6 files changed

+147
-76
lines changed

6 files changed

+147
-76
lines changed

‎src/jrd/Relation.cpp‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,14 @@ Cached::Index* RelationPermanent::lookupIndex(thread_db* tdbb, MetaId id, Object
553553
}
554554

555555

556+
Cached::Index* RelationPermanent::ensureIndex(thread_db* tdbb, MetaId id)
557+
{
558+
auto* idp = rel_indices.ensurePermanent(tdbb, id);
559+
fb_assert(idp);
560+
return idp;
561+
}
562+
563+
556564
PageNumber RelationPermanent::getIndexRootPage(thread_db* tdbb)
557565
{
558566
/**************************************
@@ -618,12 +626,6 @@ IndexVersion::IndexVersion(MemoryPool& p, Cached::Index* idp)
618626

619627
void IndexVersion::destroy(thread_db* tdbb, IndexVersion* idv)
620628
{
621-
if (idv->idv_expression_statement)
622-
idv->idv_expression_statement->release(tdbb);
623-
624-
if (idv->idv_condition_statement)
625-
idv->idv_condition_statement->release(tdbb);
626-
627629
delete idv;
628630
}
629631

@@ -997,6 +999,22 @@ FB_UINT64 IndexPermanent::makeLockId(MetaId relId, MetaId indexId)
997999
return (FB_UINT64(relId) << REL_ID_KEY_OFFSET) + indexId;
9981000
}
9991001

1002+
void IndexPermanent::releaseStatements(thread_db* tdbb)
1003+
{
1004+
if (idp_expression_statement)
1005+
{
1006+
idp_expression_statement->release(tdbb);
1007+
idp_expression_statement = nullptr;
1008+
idp_expression = nullptr;
1009+
}
1010+
if (idp_condition_statement)
1011+
{
1012+
idp_condition_statement->release(tdbb);
1013+
idp_condition_statement = nullptr;
1014+
idp_condition = nullptr;
1015+
}
1016+
}
1017+
10001018

10011019
/// jrd_rel
10021020

‎src/jrd/Relation.h‎

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "../jrd/Resources.h"
3636
#include "../common/classes/TriState.h"
3737
#include "../common/sha2/sha2.h"
38+
#include "../jrd/ods.h"
3839

3940
namespace Jrd
4041
{
@@ -460,13 +461,17 @@ class IndexPermanent : public Firebird::PermanentStorage
460461
: PermanentStorage(p),
461462
idp_relation(rel),
462463
idp_id(id)
463-
{ }
464+
{
465+
idp_expression_bid.clear();
466+
idp_condition_bid.clear();
467+
}
464468

465469
~IndexPermanent()
466470
{ }
467471

468472
static bool destroy(thread_db* tdbb, IndexPermanent* idp)
469473
{
474+
idp->releaseStatements(tdbb);
470475
return false;
471476
}
472477

@@ -488,8 +493,42 @@ class IndexPermanent : public Firebird::PermanentStorage
488493
private:
489494
RelationPermanent* idp_relation;
490495
MetaId idp_id;
496+
TraNumber idp_tranum = 0;
497+
UCHAR idp_state = 0;
491498

492499
[[noreturn]] void errIndexGone();
500+
501+
void releaseStatements(thread_db* tdbb);
502+
503+
public:
504+
void lookupIndexCode(thread_db* tdbb, Cached::Relation* relation, index_desc* idx,
505+
const Ods::index_root_page::irt_repeat* irt_desc)
506+
{
507+
if ((irt_desc->getState() != idp_state) || (irt_desc->getTransaction() != idp_tranum))
508+
refreshIndexCode(tdbb, relation, idx, irt_desc);
509+
510+
idx->idx_condition_node = idp_condition;
511+
idx->idx_condition_statement = idp_condition_statement;
512+
513+
idx->idx_expression_node = idp_expression;
514+
idx->idx_expression_statement = idp_expression_statement;
515+
memcpy(&idx->idx_expression_desc, &idp_expression_desc, sizeof(struct dsc));
516+
}
517+
518+
void refreshIndexCode(thread_db* tdbb, Cached::Relation* relation,
519+
index_desc* idx, const Ods::index_root_page::irt_repeat* irt_desc);
520+
521+
private:
522+
Firebird::Mutex idp_code_mutex; // Delays concurrent threads till the end of code refresh
523+
524+
bid idp_expression_bid;
525+
ValueExprNode* idp_expression = nullptr; // node tree for index expression
526+
Statement* idp_expression_statement = nullptr; // statement for index expression evaluation
527+
dsc idp_expression_desc; // descriptor for expression result
528+
529+
bid idp_condition_bid;
530+
BoolExprNode* idp_condition = nullptr; // node tree for index condition
531+
Statement* idp_condition_statement = nullptr; // statement for index condition evaluation
493532
};
494533

495534

@@ -561,13 +600,6 @@ class IndexVersion final : public ObjectBase
561600
SSHORT idv_type = 0;
562601
QualifiedName idv_foreignKey; // FOREIGN RELATION NAME
563602
IndexStatus idv_active = MET_index_state_unknown;
564-
565-
public:
566-
ValueExprNode* idv_expression = nullptr; // node tree for index expression
567-
Statement* idv_expression_statement = nullptr; // statement for index expression evaluation
568-
dsc idv_expression_desc; // descriptor for expression result
569-
BoolExprNode* idv_condition = nullptr; // node tree for index condition
570-
Statement* idv_condition_statement = nullptr; // statement for index condition evaluation
571603
};
572604

573605

@@ -792,6 +824,7 @@ class RelationPermanent : public Firebird::PermanentStorage
792824
IndexVersion* lookup_index(thread_db* tdbb, const QualifiedName& name, ObjectBase::Flag flags);
793825
Cached::Index* lookupIndex(thread_db* tdbb, MetaId id, ObjectBase::Flag flags);
794826
Cached::Index* lookupIndex(thread_db* tdbb, const QualifiedName& name, ObjectBase::Flag flags);
827+
Cached::Index* ensureIndex(thread_db* tdbb, MetaId id);
795828

796829
void newIndexVersion(thread_db* tdbb, MetaId id, ObjectBase::Flag scanType)
797830
{

‎src/jrd/btr.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,6 @@ bool BTR_activate_index(thread_db* tdbb, Cached::Relation* relation, MetaId id)
12591259

12601260
jrd_tra* tra = tdbb->getTransaction();
12611261
fb_assert(tra);
1262-
TraNumber descTrans = irt_desc->getTransaction();
12631262

12641263
if (tra)
12651264
{
@@ -1374,7 +1373,9 @@ bool BTR_description(thread_db* tdbb, Cached::Relation* relation, const index_ro
13741373
ISC_STATUS error = 0;
13751374
if (idx->idx_flags & (idx_expression | idx_condition))
13761375
{
1377-
MET_lookup_index_code(tdbb, relation, idx);
1376+
auto* idp = relation->ensureIndex(tdbb, idx->idx_id);
1377+
if (idp)
1378+
idp->lookupIndexCode(tdbb, relation, idx, irt_desc);
13781379

13791380
if (idx->idx_flags & idx_expression && !idx->idx_expression_node)
13801381
{

‎src/jrd/met.epp‎

Lines changed: 79 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,7 +2149,8 @@ ElementBase::ReturnedId MetadataCache::lookup_index_name(thread_db* tdbb, const
21492149
}
21502150

21512151

2152-
void MET_lookup_index_code(thread_db* tdbb, Cached::Relation* relation, index_desc* idx)
2152+
void IndexPermanent::refreshIndexCode(thread_db* tdbb, Cached::Relation* relation, index_desc* idx,
2153+
const Ods::index_root_page::irt_repeat* irt_desc)
21532154
{
21542155
/***********************************************
21552156
*
@@ -2163,15 +2164,85 @@ void MET_lookup_index_code(thread_db* tdbb, Cached::Relation* relation, index_de
21632164
**************************************/
21642165
SET_TDBB(tdbb);
21652166

2166-
IndexVersion* idv = relation->lookup_index(tdbb, idx->idx_id, CacheFlag::AUTOCREATE);
2167-
if (idv)
2167+
MutexLockGuard g(idp_code_mutex, FB_FUNCTION);
2168+
2169+
if ((irt_desc->getState() != idp_state) || (irt_desc->getTransaction() != idp_tranum))
21682170
{
2169-
idx->idx_condition_node = idv->idv_condition;
2170-
idx->idx_condition_statement = idv->idv_condition_statement;
2171+
Database* dbb = tdbb->getDatabase();
2172+
Attachment* attachment = tdbb->getAttachment();
2173+
2174+
bid expression, condition;
2175+
expression.clear();
2176+
condition.clear();
2177+
2178+
MetaId relId = relation->getId();
2179+
2180+
Jrd::ContextPoolHolder context(tdbb, dbb->dbb_permanent);
2181+
2182+
AUTO_HANDLE(handle);
2183+
FOR(REQUEST_HANDLE handle) // Use system transaction
2184+
IND IN RDB$INDICES
2185+
CROSS REL IN RDB$RELATIONS
2186+
OVER RDB$RELATION_NAME, RDB$SCHEMA_NAME
2187+
WITH IND.RDB$INDEX_ID EQ getId() + 1
2188+
AND REL.RDB$RELATION_ID EQ relId
2189+
{
2190+
if (!IND.RDB$EXPRESSION_BLR.NULL)
2191+
expression = IND.RDB$EXPRESSION_BLR;
2192+
if (!IND.RDB$CONDITION_BLR.NULL)
2193+
condition = IND.RDB$CONDITION_BLR;
2194+
}
2195+
END_FOR
2196+
2197+
if ((expression.hasData() && expression != idp_expression_bid) ||
2198+
(condition.hasData() && condition != idp_condition_bid))
2199+
{
2200+
releaseStatements(tdbb);
2201+
2202+
idp_expression_bid = expression;
2203+
idp_condition_bid = condition;
2204+
2205+
if (expression.hasData())
2206+
{
2207+
MemoryPool* stmtPool = dbb->createPool();
2208+
try
2209+
{
2210+
Jrd::ContextPoolHolder context(tdbb, stmtPool);
2211+
2212+
CompilerScratch* csb = nullptr;
2213+
Cleanup cc([csb]() {delete csb;});
2214+
MET_parse_blob(tdbb, &relation->getName().schema, relation, &expression, &csb, nullptr, false, false);
2215+
idp_expression_statement = Statement::makeValueExpression(tdbb, idp_expression, idp_expression_desc, csb, false);
2216+
}
2217+
catch (const Exception&)
2218+
{
2219+
dbb->deletePool(stmtPool);
2220+
throw;
2221+
}
2222+
}
2223+
2224+
if (condition.hasData())
2225+
{
2226+
MemoryPool* stmtPool = dbb->createPool();
2227+
try
2228+
{
2229+
Jrd::ContextPoolHolder context(tdbb, stmtPool);
2230+
2231+
CompilerScratch* csb = nullptr;
2232+
Cleanup cc([csb]() {delete csb;});
2233+
MET_parse_blob(tdbb, &relation->getName().schema, relation, &condition, &csb, nullptr, false, false);
2234+
idp_condition_statement = Statement::makeBoolExpression(tdbb, idp_condition, csb, false);
2235+
}
2236+
catch (const Exception&)
2237+
{
2238+
dbb->deletePool(stmtPool);
2239+
throw;
2240+
}
2241+
}
2242+
}
21712243

2172-
idx->idx_expression_node = idv->idv_expression;
2173-
idx->idx_expression_statement = idv->idv_expression_statement;
2174-
memcpy(&idx->idx_expression_desc, &idv->idv_expression_desc, sizeof(struct dsc));
2244+
idp_state = irt_desc->getState();
2245+
idp_tranum = irt_desc->getTransaction();
21752246
}
21762247
}
21772248

@@ -4878,9 +4949,6 @@ ScanResult IndexVersion::scan(thread_db* tdbb, ObjectBase::Flag flags)
48784949
Database* dbb = tdbb->getDatabase();
48794950
jrd_tra* transaction = attachment->getMetaTransaction(tdbb);
48804951

4881-
bid expression, condition;
4882-
expression.clear();
4883-
condition.clear();
48844952
MetaId relId = getPermanent()->getRelation()->getId();
48854953
bool found = false;
48864954

@@ -4903,11 +4971,6 @@ ScanResult IndexVersion::scan(thread_db* tdbb, ObjectBase::Flag flags)
49034971
idv_foreignKey = QualifiedName(IND.RDB$FOREIGN_KEY, IND.RDB$FOREIGN_KEY_SCHEMA_NAME);
49044972

49054973
idv_active = MetadataCache::getIndexStatus(IND.RDB$INDEX_INACTIVE.NULL, IND.RDB$INDEX_INACTIVE);
4906-
4907-
if (!IND.RDB$EXPRESSION_BLR.NULL)
4908-
expression = IND.RDB$EXPRESSION_BLR;
4909-
if (!IND.RDB$CONDITION_BLR.NULL)
4910-
condition = IND.RDB$CONDITION_BLR;
49114974
}
49124975
END_FOR
49134976

@@ -4917,46 +4980,6 @@ ScanResult IndexVersion::scan(thread_db* tdbb, ObjectBase::Flag flags)
49174980
return ScanResult::MISS;
49184981
}
49194982

4920-
auto* relation = (expression.hasData() || condition.hasData()) ?
4921-
MetadataCache::getPerm<Cached::Relation>(tdbb, relId, CacheFlag::AUTOCREATE) : nullptr;
4922-
4923-
if (expression.hasData())
4924-
{
4925-
MemoryPool* stmtPool = dbb->createPool();
4926-
try
4927-
{
4928-
Jrd::ContextPoolHolder context(tdbb, stmtPool);
4929-
4930-
CompilerScratch* csb = nullptr;
4931-
Cleanup cc([csb]() {delete csb;});
4932-
MET_parse_blob(tdbb, &relation->getName().schema, relation, &expression, &csb, nullptr, false, false);
4933-
idv_expression_statement = Statement::makeValueExpression(tdbb, idv_expression, idv_expression_desc, csb, false);
4934-
}
4935-
catch (const Exception&)
4936-
{
4937-
dbb->deletePool(stmtPool);
4938-
throw;
4939-
}
4940-
}
4941-
4942-
if (condition.hasData())
4943-
{
4944-
MemoryPool* stmtPool = dbb->createPool();
4945-
try
4946-
{
4947-
Jrd::ContextPoolHolder context(tdbb, stmtPool);
4948-
4949-
CompilerScratch* csb = nullptr;
4950-
Cleanup cc([csb]() {delete csb;});
4951-
MET_parse_blob(tdbb, &relation->getName().schema, relation, &condition, &csb, nullptr, false, false);
4952-
idv_condition_statement = Statement::makeBoolExpression(tdbb, idv_condition, csb, false);
4953-
}
4954-
catch (const Exception&)
4955-
{
4956-
dbb->deletePool(stmtPool);
4957-
throw;
4958-
}
4959-
}
49604983

49614984
return found ? ScanResult::COMPLETE : ScanResult::MISS;
49624985
}

‎src/jrd/met_proto.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ bool MET_load_generator(Jrd::thread_db*, Jrd::GeneratorItem&, bool* sysGen = 0,
112112
SLONG MET_lookup_generator(Jrd::thread_db*, const Jrd::QualifiedName&, bool* sysGen = 0, SLONG* step = 0);
113113
bool MET_lookup_generator_id(Jrd::thread_db*, SLONG, Jrd::QualifiedName&, bool* sysGen = 0);
114114
void MET_update_generator_increment(Jrd::thread_db* tdbb, SLONG gen_id, SLONG step);
115-
void MET_lookup_index_code(Jrd::thread_db* tdbb, Jrd::Cached::Relation* relation, Jrd::index_desc* idx);
116115
bool MET_lookup_index_expr_cond_blr(Jrd::thread_db* tdbb, const Jrd::QualifiedName& index_name,
117116
Jrd::bid& expr_blob_id, Jrd::bid& cond_blob_id);
118117

‎src/jrd/ods.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,6 @@ inline void index_root_page::irt_repeat::setRoot(ULONG rootPage)
510510
fb_assert(getState() == irt_in_progress || getState() == irt_normal);
511511
fb_assert(rootPage);
512512

513-
irt_transaction = 0;
514513
irt_page_num = rootPage;
515514
irt_page_space_id = 0;
516515
setState(irt_normal);
@@ -578,7 +577,6 @@ inline void index_root_page::irt_repeat::setNormal()
578577
fb_assert(irt_page_num);
579578
fb_assert(irt_transaction);
580579

581-
irt_transaction = 0;
582580
setState(irt_normal);
583581
}
584582

@@ -589,7 +587,6 @@ inline void index_root_page::irt_repeat::setNormal(ULONG rootPage)
589587
fb_assert(!irt_page_num);
590588
fb_assert(rootPage);
591589

592-
irt_transaction = 0;
593590
irt_page_num = rootPage;
594591
irt_page_space_id = 0;
595592
setState(irt_normal);

0 commit comments

Comments
 (0)