From 26a149c55a1ebc5921ab2f5ee682dc345f32b893 Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sun, 25 Sep 2022 10:49:11 +0300 Subject: [PATCH] Fixed #3812: Query with SP doesn't accept explicit plan --- src/dsql/gen.cpp | 6 +- src/dsql/metd.epp | 35 ++-- src/dsql/metd_proto.h | 4 +- src/include/firebird/impl/msg/jrd.h | 9 +- src/include/gen/Firebird.pas | 1 + src/jrd/RecordSourceNodes.cpp | 276 ++++++++++++++++------------ src/jrd/RecordSourceNodes.h | 23 +-- src/jrd/optimizer/Optimizer.cpp | 10 +- src/jrd/par.cpp | 82 +++++++-- 9 files changed, 269 insertions(+), 177 deletions(-) diff --git a/src/dsql/gen.cpp b/src/dsql/gen.cpp index 63536cff5d..9ac0107e19 100644 --- a/src/dsql/gen.cpp +++ b/src/dsql/gen.cpp @@ -465,10 +465,10 @@ static void gen_plan(DsqlCompilerScratch* dsqlScratch, const PlanNode* planNode) // stuff the relation -- the relation id itself is redundant except // when there is a need to differentiate the base tables of views - // ASF: node->dsqlRecordSourceNode may be NULL, and then a BLR error will happen. + // ASF: node->recordSourceNode may be NULL, and then a BLR error will happen. // Example command: select * from (select * from t1) a plan (a natural); - if (node->dsqlRecordSourceNode) - node->dsqlRecordSourceNode->genBlr(dsqlScratch); + if (node->recordSourceNode) + node->recordSourceNode->genBlr(dsqlScratch); // now stuff the access method for this stream diff --git a/src/dsql/metd.epp b/src/dsql/metd.epp index bfb47f644e..fbafeb7809 100644 --- a/src/dsql/metd.epp +++ b/src/dsql/metd.epp @@ -1524,8 +1524,9 @@ dsql_rel* METD_get_view_base(jrd_tra* transaction, DsqlCompilerScratch* dsqlScra } -dsql_rel* METD_get_view_relation(jrd_tra* transaction, DsqlCompilerScratch* dsqlScratch, - const char* view_name, const char* relation_or_alias) +bool METD_get_view_relation(jrd_tra* transaction, DsqlCompilerScratch* dsqlScratch, + const Jrd::MetaName& view_name, const Jrd::MetaName& relation_or_alias, + dsql_rel*& relation, dsql_prc*& procedure) { /************************************** * @@ -1543,32 +1544,36 @@ dsql_rel* METD_get_view_relation(jrd_tra* transaction, DsqlCompilerScratch* dsql validateTransaction(transaction); - dsql_rel* relation = NULL; - AutoCacheRequest handle(tdbb, irq_view, IRQ_REQUESTS); FOR(REQUEST_HANDLE handle TRANSACTION_HANDLE transaction) - X IN RDB$VIEW_RELATIONS WITH X.RDB$VIEW_NAME EQ view_name + X IN RDB$VIEW_RELATIONS WITH X.RDB$VIEW_NAME EQ view_name.c_str() { fb_utils::exact_name(X.RDB$CONTEXT_NAME); fb_utils::exact_name(X.RDB$RELATION_NAME); - if (!strcmp(X.RDB$RELATION_NAME, relation_or_alias) || - !strcmp(X.RDB$CONTEXT_NAME, relation_or_alias)) + if (relation_or_alias == X.RDB$RELATION_NAME || + relation_or_alias == X.RDB$CONTEXT_NAME) { - relation = METD_get_relation(transaction, dsqlScratch, X.RDB$RELATION_NAME); - return relation; + if ( (relation = METD_get_relation(transaction, dsqlScratch, X.RDB$RELATION_NAME)) ) + return true; + + const QualifiedName procName(X.RDB$RELATION_NAME, + X.RDB$PACKAGE_NAME.NULL ? nullptr : X.RDB$PACKAGE_NAME); + + if ( (procedure = METD_get_procedure(transaction, dsqlScratch, procName)) ) + return true; } - relation = METD_get_view_relation(transaction, dsqlScratch, X.RDB$RELATION_NAME, - relation_or_alias); - - if (relation) - return relation; + if (METD_get_view_relation(transaction, dsqlScratch, X.RDB$RELATION_NAME, + relation_or_alias, relation, procedure)) + { + return true; + } } END_FOR - return NULL; + return false; } diff --git a/src/dsql/metd_proto.h b/src/dsql/metd_proto.h index fe9faa2c36..f2130052ec 100644 --- a/src/dsql/metd_proto.h +++ b/src/dsql/metd_proto.h @@ -67,7 +67,7 @@ Jrd::dsql_rel* METD_get_relation(Jrd::jrd_tra*, Jrd::DsqlCompilerScratch*, const bool METD_get_type(Jrd::jrd_tra*, const Jrd::MetaName&, const char*, SSHORT*); Jrd::dsql_rel* METD_get_view_base(Jrd::jrd_tra*, Jrd::DsqlCompilerScratch*, const char* view_name, Jrd::MetaNamePairMap& fields); -Jrd::dsql_rel* METD_get_view_relation(Jrd::jrd_tra*, Jrd::DsqlCompilerScratch*, const char* view_name, - const char* relation_or_alias); +bool METD_get_view_relation(Jrd::jrd_tra*, Jrd::DsqlCompilerScratch*, const Jrd::MetaName& view_name, + const Jrd::MetaName& relation_or_alias, Jrd::dsql_rel*& relation, Jrd::dsql_prc*& procedure); #endif // DSQL_METD_PROTO_H diff --git a/src/include/firebird/impl/msg/jrd.h b/src/include/firebird/impl/msg/jrd.h index 6c12691e1b..1b8ea1a9fb 100644 --- a/src/include/firebird/impl/msg/jrd.h +++ b/src/include/firebird/impl/msg/jrd.h @@ -315,9 +315,9 @@ FB_IMPL_MSG(JRD, 313, dsql_shadow_number_err, -598, "HY", "000", "Shadow number FB_IMPL_MSG(JRD, 314, dsql_token_unk_err, -104, "42", "000", "Token unknown - line @1, column @2") FB_IMPL_MSG(JRD, 315, dsql_no_relation_alias, -204, "42", "S02", "there is no alias or table named @1 at this scope level") FB_IMPL_MSG(JRD, 316, indexname, -204, "42", "000", "there is no index @1 for table @2") -FB_IMPL_MSG(JRD, 317, no_stream_plan, -281, "HY", "000", "table @1 is not referenced in plan") -FB_IMPL_MSG(JRD, 318, stream_twice, -282, "HY", "000", "table @1 is referenced more than once in plan; use aliases to distinguish") -FB_IMPL_MSG(JRD, 319, stream_not_found, -283, "HY", "000", "table @1 is referenced in the plan but not the from list") +FB_IMPL_MSG(JRD, 317, no_stream_plan, -281, "HY", "000", "table or procedure @1 is not referenced in plan") +FB_IMPL_MSG(JRD, 318, stream_twice, -282, "HY", "000", "table or procedure @1 is referenced more than once in plan; use aliases to distinguish") +FB_IMPL_MSG(JRD, 319, stream_not_found, -283, "HY", "000", "table or procedure @1 is referenced in the plan but not the from list") FB_IMPL_MSG(JRD, 320, collation_requires_text, -204, "22", "021", "Invalid use of CHARACTER SET or COLLATE") FB_IMPL_MSG(JRD, 321, dsql_domain_not_found, -901, "42", "000", "Specified domain or source column @1 does not exist") FB_IMPL_MSG(JRD, 322, index_unused, -284, "42", "000", "index @1 cannot be used in the specified plan") @@ -337,7 +337,7 @@ FB_IMPL_MSG(JRD, 335, invalid_direction, -827, "42", "000", "invalid direction f FB_IMPL_MSG(JRD, 336, dsql_var_conflict, -901, "HY", "000", "variable @1 conflicts with parameter in same procedure") FB_IMPL_MSG(JRD, 337, dsql_no_blob_array, -607, "HY", "000", "Array/BLOB/DATE data types not allowed in arithmetic") FB_IMPL_MSG(JRD, 338, dsql_base_table, -155, "42", "000", "@1 is not a valid base table of the specified view") -FB_IMPL_MSG(JRD, 339, duplicate_base_table, -282, "42", "000", "table @1 is referenced twice in view; use an alias to distinguish") +FB_IMPL_MSG(JRD, 339, duplicate_base_table, -282, "42", "000", "table or procedure @1 is referenced twice in view; use an alias to distinguish") FB_IMPL_MSG(JRD, 340, view_alias, -282, "42", "000", "view @1 has more than one base table; use aliases to distinguish") FB_IMPL_MSG(JRD, 341, index_root_page_full, -904, "54", "000", "cannot add index, index root page is full.") FB_IMPL_MSG(JRD, 342, dsql_blob_type_unknown, -204, "42", "000", "BLOB SUB_TYPE @1 is not defined") @@ -960,3 +960,4 @@ FB_IMPL_MSG(JRD, 958, quoted_str_bad, -901, "22", "024", "Invalid text <@1> afte FB_IMPL_MSG(JRD, 959, quoted_str_miss, -901, "22", "024", "Missing terminating quote <@1> in the end of quoted string") FB_IMPL_MSG(JRD, 960, wrong_shmem_ver, -902, "08", "006", "@1: inconsistent shared memory type/version; found @2, expected @3") FB_IMPL_MSG(JRD, 961, wrong_shmem_bitness, -902, "08", "006", "@1-bit engine can't open database already opened by @2-bit engine") +FB_IMPL_MSG(JRD, 962, wrong_proc_plan, -281, "HY", "000", "Procedures cannot specify access type other than NATURAL in the plan") diff --git a/src/include/gen/Firebird.pas b/src/include/gen/Firebird.pas index f0da08405f..6ec1ac4ce1 100644 --- a/src/include/gen/Firebird.pas +++ b/src/include/gen/Firebird.pas @@ -5298,6 +5298,7 @@ const isc_quoted_str_miss = 335545279; isc_wrong_shmem_ver = 335545280; isc_wrong_shmem_bitness = 335545281; + isc_wrong_proc_plan = 335545282; isc_gfix_db_name = 335740929; isc_gfix_invalid_sw = 335740930; isc_gfix_incmp_sw = 335740932; diff --git a/src/jrd/RecordSourceNodes.cpp b/src/jrd/RecordSourceNodes.cpp index 82225a27b0..ea9749868a 100644 --- a/src/jrd/RecordSourceNodes.cpp +++ b/src/jrd/RecordSourceNodes.cpp @@ -44,7 +44,6 @@ using namespace Jrd; static RecordSourceNode* dsqlPassRelProc(DsqlCompilerScratch* dsqlScratch, RecordSourceNode* source); static MapNode* parseMap(thread_db* tdbb, CompilerScratch* csb, StreamType stream, bool parseHeader); -static int strcmpSpace(const char* p, const char* q); static void processSource(thread_db* tdbb, CompilerScratch* csb, RseNode* rse, RecordSourceNode* source, BoolExprNode** boolean, RecordSourceNodeStack& stack); static void processMap(thread_db* tdbb, CompilerScratch* csb, MapNode* map, Format** inputFormat); @@ -187,7 +186,7 @@ PlanNode* PlanNode::dsqlPass(DsqlCompilerScratch* dsqlScratch) node->accessType->items = accessType->items; } - node->relationNode = relationNode; + node->recordSourceNode = recordSourceNode; for (NestConst* i = subNodes.begin(); i != subNodes.end(); ++i) node->subNodes.add((*i)->dsqlPass(dsqlScratch)); @@ -203,21 +202,19 @@ PlanNode* PlanNode::dsqlPass(DsqlCompilerScratch* dsqlScratch) { RelationSourceNode* relNode = FB_NEW_POOL(pool) RelationSourceNode(pool); relNode->dsqlContext = context; - node->dsqlRecordSourceNode = relNode; + node->recordSourceNode = relNode; } else if (context->ctx_procedure) { - // ASF: Note that usage of procedure name in a PLAN clause causes errors when - // parsing the BLR. ProcedureSourceNode* procNode = FB_NEW_POOL(pool) ProcedureSourceNode(pool); procNode->dsqlContext = context; - node->dsqlRecordSourceNode = procNode; + node->recordSourceNode = procNode; } //// TODO: LocalTableSourceNode - // ASF: I think it's a error to let node->dsqlRecordSourceNode be NULL here, but it happens + // ASF: I think it's a error to let node->recordSourceNode be NULL here, but it happens // at least since v2.5. See gen.cpp/gen_plan for more information. - ///fb_assert(node->dsqlRecordSourceNode); + ///fb_assert(node->recordSourceNode); } return node; @@ -253,27 +250,37 @@ dsql_ctx* PlanNode::dsqlPassAliasList(DsqlCompilerScratch* dsqlScratch) { // This must be a VIEW ObjectsArray::iterator startArg = arg; - dsql_rel* relation = context->ctx_relation; + dsql_rel* viewRelation = context->ctx_relation; + + dsql_rel* relation = nullptr; + dsql_prc* procedure = nullptr; // find the base table using the specified alias list, skipping the first one // since we already matched it to the context. - for (; arg != end; ++arg, --aliasCount) + for (; arg != end; ++arg) { - relation = METD_get_view_relation(dsqlScratch->getTransaction(), - dsqlScratch, relation->rel_name.c_str(), arg->c_str()); + if (!METD_get_view_relation(dsqlScratch->getTransaction(), + dsqlScratch, viewRelation->rel_name, *arg, + relation, procedure)) + { + break; + }; + + --aliasCount; if (!relation) break; } // Found base relation - if (aliasCount == 0 && relation) + if (aliasCount == 0 && (relation || procedure)) { // AB: Pretty ugly huh? // make up a dummy context to hold the resultant relation. dsql_ctx* newContext = FB_NEW_POOL(dsqlScratch->getPool()) dsql_ctx(dsqlScratch->getPool()); newContext->ctx_context = context->ctx_context; newContext->ctx_relation = relation; + newContext->ctx_procedure = procedure; // Concatenate all the contexts to form the alias name. // Calculate the length leaving room for spaces. @@ -327,7 +334,7 @@ dsql_ctx* PlanNode::dsqlPassAliasList(DsqlCompilerScratch* dsqlScratch) dsql_ctx* PlanNode::dsqlPassAlias(DsqlCompilerScratch* dsqlScratch, DsqlContextStack& stack, const MetaName& alias) { - dsql_ctx* relation_context = NULL; + dsql_ctx* result_context = nullptr; DEV_BLKCHK(dsqlScratch, dsql_type_req); @@ -351,14 +358,15 @@ dsql_ctx* PlanNode::dsqlPassAlias(DsqlCompilerScratch* dsqlScratch, DsqlContextS // If an unnamed derived table and empty alias. if (context->ctx_rse && !context->ctx_relation && !context->ctx_procedure && alias.isEmpty()) - relation_context = context; + result_context = context; // Check for matching relation name; aliases take priority so // save the context in case there is an alias of the same name. // Also to check that there is no self-join in the query. - if (context->ctx_relation && context->ctx_relation->rel_name == alias) + if ((context->ctx_relation && context->ctx_relation->rel_name == alias) || + (context->ctx_procedure && context->ctx_procedure->prc_name.identifier == alias)) { - if (relation_context) + if (result_context) { // the table %s is referenced twice; use aliases to differentiate ERRD_post(Arg::Gds(isc_sqlerr) << Arg::Num(-104) << @@ -366,11 +374,11 @@ dsql_ctx* PlanNode::dsqlPassAlias(DsqlCompilerScratch* dsqlScratch, DsqlContextS Arg::Gds(isc_dsql_self_join) << alias); } - relation_context = context; + result_context = context; } } - return relation_context; + return result_context; } @@ -885,7 +893,7 @@ RecordSource* RelationSourceNode::compile(thread_db* tdbb, Optimizer* opt, bool // Parse an procedural view reference. ProcedureSourceNode* ProcedureSourceNode::parse(thread_db* tdbb, CompilerScratch* csb, - const SSHORT blrOp) + const SSHORT blrOp, bool parseContext) { SET_TDBB(tdbb); @@ -992,16 +1000,22 @@ ProcedureSourceNode* ProcedureSourceNode::parse(thread_db* tdbb, CompilerScratch node->procedure = procedure; node->isSubRoutine = procedure->isSubRoutine(); node->procedureId = node->isSubRoutine ? 0 : procedure->getId(); - node->stream = PAR_context(csb, &node->context); - csb->csb_rpt[node->stream].csb_procedure = procedure; - csb->csb_rpt[node->stream].csb_alias = aliasString; + if (parseContext) + { + node->stream = PAR_context(csb, &node->context); - PAR_procedure_parms(tdbb, csb, procedure, node->in_msg.getAddress(), - node->sourceList.getAddress(), node->targetList.getAddress(), true); + csb->csb_rpt[node->stream].csb_procedure = procedure; + csb->csb_rpt[node->stream].csb_alias = aliasString; - if (csb->collectingDependencies()) - PAR_dependency(tdbb, csb, node->stream, (SSHORT) -1, ""); + PAR_procedure_parms(tdbb, csb, procedure, node->in_msg.getAddress(), + node->sourceList.getAddress(), node->targetList.getAddress(), true); + + if (csb->collectingDependencies()) + PAR_dependency(tdbb, csb, node->stream, (SSHORT) -1, ""); + } + else + delete aliasString; return node; } @@ -3017,14 +3031,20 @@ void RseNode::planCheck(const CompilerScratch* csb) const for (const auto node : rse_relations) { - if (nodeIs(node)) + if (nodeIs(node) || nodeIs(node)) { - const StreamType stream = node->getStream(); + const auto stream = node->getStream(); - if (!(csb->csb_rpt[stream].csb_plan)) + const auto relation = csb->csb_rpt[stream].csb_relation; + const auto procedure = csb->csb_rpt[stream].csb_procedure; + fb_assert(relation || procedure); + + if (!csb->csb_rpt[stream].csb_plan) { - ERR_post(Arg::Gds(isc_no_stream_plan) << - Arg::Str(csb->csb_rpt[stream].csb_relation->rel_name)); + const auto name = relation ? relation->rel_name : + procedure ? procedure->getName().toString() : ""; + + ERR_post(Arg::Gds(isc_no_stream_plan) << Arg::Str(name)); } } else if (const auto rse = nodeAs(node)) @@ -3046,50 +3066,77 @@ void RseNode::planSet(CompilerScratch* csb, PlanNode* plan) if (plan->type != PlanNode::TYPE_RETRIEVE) return; - const jrd_rel* viewRelation = NULL; - const jrd_rel* planRelation = plan->relationNode->relation; - const char* planAlias = plan->relationNode->alias.c_str(); + // Find the tail for the relation/procedure specified in the plan - // find the tail for the relation specified in the RseNode + const auto stream = plan->recordSourceNode->getStream(); + auto tail = &csb->csb_rpt[stream]; - const StreamType stream = plan->relationNode->getStream(); - CompilerScratch::csb_repeat* tail = &csb->csb_rpt[stream]; + string planAlias; - // if the plan references a view, find the real base relation + jrd_rel* planRelation = nullptr; + if (const auto relationNode = nodeAs(plan->recordSourceNode)) + { + planRelation = relationNode->relation; + planAlias = relationNode->alias; + } + + jrd_prc* planProcedure = nullptr; + if (const auto procedureNode = nodeAs(plan->recordSourceNode)) + { + planProcedure = procedureNode->procedure; + planAlias = procedureNode->alias; + } + + fb_assert(planRelation || planProcedure); + + const auto name = planRelation ? planRelation->rel_name : + planProcedure ? planProcedure->getName().toString() : ""; + + // If the plan references a view, find the real base relation // we are interested in by searching the view map - StreamType* map = NULL; + StreamType* map = nullptr; + jrd_rel* viewRelation = nullptr; + jrd_prc* viewProcedure = nullptr; if (tail->csb_map) { - const TEXT* p = planAlias; + auto tailName = tail->csb_relation ? tail->csb_relation->rel_name : + tail->csb_procedure ? tail->csb_procedure->getName().toString() : ""; - // if the user has specified an alias, skip past it to find the alias + // If the user has specified an alias, skip past it to find the alias // for the base table (if multiple aliases are specified) - if (p && *p && - ((tail->csb_relation && !strcmpSpace(tail->csb_relation->rel_name.c_str(), p)) || - (tail->csb_alias && !strcmpSpace(tail->csb_alias->c_str(), p)))) - { - while (*p && *p != ' ') - p++; - if (*p == ' ') - p++; + auto tailAlias = tail->csb_alias ? *tail->csb_alias : ""; + + if (planAlias.hasData()) + { + const auto spacePos = planAlias.find_first_of(' '); + const auto subAlias = planAlias.substr(0, spacePos); + + if (tailName == subAlias || tailAlias == subAlias) + { + planAlias = planAlias.substr(spacePos); + planAlias.ltrim(); + } } - // loop through potentially a stack of views to find the appropriate base table + // Loop through potentially a stack of views to find the appropriate base table StreamType* mapBase; - while ( (mapBase = tail->csb_map) ) { map = mapBase; tail = &csb->csb_rpt[*map]; viewRelation = tail->csb_relation; + viewProcedure = tail->csb_procedure; - // if the plan references the view itself, make sure that - // the view is on a single table; if it is, fix up the plan - // to point to the base relation + // If the plan references the view itself, make sure that + // the view is on a single table. If it is, fix up the plan + // to point to the base relation. - if (viewRelation->rel_id == planRelation->rel_id) + if ((viewRelation && planRelation && + viewRelation->rel_id == planRelation->rel_id) || + (viewProcedure && planProcedure && + viewProcedure->getId() == planProcedure->getId())) { if (!mapBase[2]) { @@ -3099,41 +3146,47 @@ void RseNode::planSet(CompilerScratch* csb, PlanNode* plan) else { // view %s has more than one base relation; use aliases to distinguish - ERR_post(Arg::Gds(isc_view_alias) << Arg::Str(planRelation->rel_name)); + ERR_post(Arg::Gds(isc_view_alias) << Arg::Str(name)); } break; } - viewRelation = NULL; + viewRelation = nullptr; + viewProcedure = nullptr; - // if the user didn't specify an alias (or didn't specify one + // If the user didn't specify an alias (or didn't specify one // for this level), check to make sure there is one and only one // base relation in the table which matches the plan relation - if (!*p) + if (planAlias.isEmpty()) { - const jrd_rel* duplicateRelation = NULL; - StreamType* duplicateMap = mapBase; + auto duplicateMap = mapBase; + MetaName duplicateName; - map = NULL; + map = nullptr; for (duplicateMap++; *duplicateMap; ++duplicateMap) { - CompilerScratch::csb_repeat* duplicateTail = &csb->csb_rpt[*duplicateMap]; - const jrd_rel* relation = duplicateTail->csb_relation; + const auto duplicateTail = &csb->csb_rpt[*duplicateMap]; + const auto relation = duplicateTail->csb_relation; + const auto procedure = duplicateTail->csb_procedure; - if (relation && relation->rel_id == planRelation->rel_id) + if ((relation && planRelation && + relation->rel_id == planRelation->rel_id) || + (procedure && planProcedure && + procedure->getId() == planProcedure->getId())) { - if (duplicateRelation) + if (duplicateName.hasData()) { // table %s is referenced twice in view; use an alias to distinguish ERR_post(Arg::Gds(isc_duplicate_base_table) << - Arg::Str(duplicateRelation->rel_name)); + Arg::Str(duplicateName)); } else { - duplicateRelation = relation; + duplicateName = relation ? relation->rel_name : + procedure ? procedure->getName().toString() : ""; map = duplicateMap; tail = duplicateTail; } @@ -3143,76 +3196,75 @@ void RseNode::planSet(CompilerScratch* csb, PlanNode* plan) break; } - // look through all the base relations for a match + // Look through all the base relations for a match map = mapBase; for (map++; *map; map++) { tail = &csb->csb_rpt[*map]; - const jrd_rel* relation = tail->csb_relation; - // match the user-supplied alias with the alias supplied - // with the view definition; failing that, try the base - // table name itself + tailName = tail->csb_relation ? tail->csb_relation->rel_name : + tail->csb_procedure ? tail->csb_procedure->getName().toString() : ""; - // CVC: I found that "relation" can be NULL, too. This may be an - // indication of a logic flaw while parsing the user supplied SQL plan - // and not an oversight here. It's hard to imagine a csb->csb_rpt with - // a NULL relation. See exe.h for CompilerScratch struct and its inner csb_repeat struct. + // Match the user-supplied alias with the alias supplied + // with the view definition. Failing that, try the base + // table name itself. - if ((tail->csb_alias && !strcmpSpace(tail->csb_alias->c_str(), p)) || - (relation && !strcmpSpace(relation->rel_name.c_str(), p))) + tailAlias = tail->csb_alias ? *tail->csb_alias : ""; + + const auto spacePos = planAlias.find_first_of(' '); + const auto subAlias = planAlias.substr(0, spacePos); + + if (tailName == subAlias || tailAlias == subAlias) { + // Skip past the alias + planAlias = planAlias.substr(spacePos); + planAlias.ltrim(); break; } } - // skip past the alias - - while (*p && *p != ' ') - p++; - - if (*p == ' ') - p++; - if (!*map) { - // table %s is referenced in the plan but not the from list - ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(planRelation->rel_name)); + // table or procedure %s is referenced in the plan but not the from list + ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(name)); } } - // fix up the relation node to point to the base relation's stream + // Fix up the relation node to point to the base relation's stream if (!map || !*map) { - // table %s is referenced in the plan but not the from list - ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(planRelation->rel_name)); + // table or procedure %s is referenced in the plan but not the from list + ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(name)); } - plan->relationNode->setStream(*map); + plan->recordSourceNode->setStream(*map); } - // make some validity checks + // Make some validity checks - if (!tail->csb_relation) + if (!tail->csb_relation && !tail->csb_procedure) { - // table %s is referenced in the plan but not the from list - ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(planRelation->rel_name)); + // table or procedure %s is referenced in the plan but not the from list + ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(name)); } - if ((tail->csb_relation->rel_id != planRelation->rel_id) && !viewRelation) + if ((tail->csb_relation && planRelation && + tail->csb_relation->rel_id != planRelation->rel_id && !viewRelation) || + (tail->csb_procedure && planProcedure && + tail->csb_procedure->getId() != planProcedure->getId() && !viewProcedure)) { - // table %s is referenced in the plan but not the from list - ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(planRelation->rel_name)); + // table or procedure %s is referenced in the plan but not the from list + ERR_post(Arg::Gds(isc_stream_not_found) << Arg::Str(name)); } - // check if we already have a plan for this stream + // Check if we already have a plan for this stream if (tail->csb_plan) { - // table %s is referenced more than once in plan; use aliases to distinguish - ERR_post(Arg::Gds(isc_stream_twice) << Arg::Str(tail->csb_relation->rel_name)); + // table or procedure %s is referenced more than once in plan; use aliases to distinguish + ERR_post(Arg::Gds(isc_stream_twice) << Arg::Str(name)); } tail->csb_plan = plan; @@ -3432,22 +3484,6 @@ static MapNode* parseMap(thread_db* tdbb, CompilerScratch* csb, StreamType strea return node; } -// Compare two strings, which could be either space-terminated or null-terminated. -static int strcmpSpace(const char* p, const char* q) -{ - for (; *p && *p != ' ' && *q && *q != ' '; p++, q++) - { - if (*p != *q) - break; - } - - if ((!*p || *p == ' ') && (!*q || *q == ' ')) - return 0; - - return (*p > *q) ? 1 : -1; -} - - // Process a single record source stream from an RseNode. // Obviously, if the source is a view, there is more work to do. static void processSource(thread_db* tdbb, CompilerScratch* csb, RseNode* rse, diff --git a/src/jrd/RecordSourceNodes.h b/src/jrd/RecordSourceNodes.h index 2546ac55a6..dab9e97a64 100644 --- a/src/jrd/RecordSourceNodes.h +++ b/src/jrd/RecordSourceNodes.h @@ -174,9 +174,8 @@ public: : PermanentStorage(pool), type(aType), accessType(NULL), - relationNode(NULL), + recordSourceNode(NULL), subNodes(pool), - dsqlRecordSourceNode(NULL), dsqlNames(NULL) { } @@ -198,9 +197,8 @@ private: public: Type const type; AccessType* accessType; - RelationSourceNode* relationNode; + RecordSourceNode* recordSourceNode; Firebird::Array > subNodes; - RecordSourceNode* dsqlRecordSourceNode; Firebird::ObjectsArray* dsqlNames; }; @@ -437,10 +435,10 @@ public: : TypedNode(pool), dsqlName(pool, aDsqlName), alias(pool), + procedure(NULL), sourceList(NULL), targetList(NULL), in_msg(NULL), - procedure(NULL), view(NULL), procedureId(0), context(0), @@ -448,7 +446,8 @@ public: { } - static ProcedureSourceNode* parse(thread_db* tdbb, CompilerScratch* csb, const SSHORT blrOp); + static ProcedureSourceNode* parse(thread_db* tdbb, CompilerScratch* csb, const SSHORT blrOp, + bool parseContext); virtual Firebird::string internalPrint(NodePrinter& printer) const; virtual RecordSourceNode* dsqlPass(DsqlCompilerScratch* dsqlScratch); @@ -492,11 +491,6 @@ public: public: QualifiedName dsqlName; Firebird::string alias; - NestConst sourceList; - NestConst targetList; - -private: - NestConst in_msg; /*** dimitr: Referencing procedures via a pointer is not currently reliable, because @@ -515,7 +509,14 @@ private: explicit unload from the metadata cache. But we don't have clearly established cache management policies yet, so I leave it for the other day. ***/ + jrd_prc* procedure; + NestConst sourceList; + NestConst targetList; + +private: + NestConst in_msg; + jrd_rel* view; USHORT procedureId; SSHORT context; diff --git a/src/jrd/optimizer/Optimizer.cpp b/src/jrd/optimizer/Optimizer.cpp index 252cebad3d..21db6f392f 100644 --- a/src/jrd/optimizer/Optimizer.cpp +++ b/src/jrd/optimizer/Optimizer.cpp @@ -410,7 +410,8 @@ namespace // If there were none indices, this is a sequential retrieval. const auto relation = tail->csb_relation; - fb_assert(relation); + if (!relation) + return; if (!tail->csb_idx) return; @@ -1528,6 +1529,8 @@ void Optimizer::checkIndices() continue; const auto relation = tail->csb_relation; + if (!relation) + return; // If there were no indices fetched at all but the user specified some, // error out using the first index specified @@ -2187,7 +2190,10 @@ void Optimizer::formRivers(const StreamList& streams, // the stream into the river fb_assert(planNode->type == PlanNode::TYPE_RETRIEVE); - const StreamType stream = planNode->relationNode->getStream(); + if (!nodeIs(planNode->recordSourceNode)) + continue; + + const auto stream = planNode->recordSourceNode->getStream(); // dimitr: the plan may contain more retrievals than the "streams" // array (some streams could already be joined to the active diff --git a/src/jrd/par.cpp b/src/jrd/par.cpp index 8c063ba184..f80b0da4c0 100644 --- a/src/jrd/par.cpp +++ b/src/jrd/par.cpp @@ -933,11 +933,11 @@ static PlanNode* par_plan(thread_db* tdbb, CompilerScratch* csb) **************************************/ SET_TDBB(tdbb); - USHORT node_type = (USHORT) csb->csb_blr_reader.getByte(); + auto blrOp = csb->csb_blr_reader.getByte(); // a join type indicates a cross of two or more streams - if (node_type == blr_join || node_type == blr_merge) + if (blrOp == blr_join || blrOp == blr_merge) { // CVC: bottleneck int count = (USHORT) csb->csb_blr_reader.getByte(); @@ -951,7 +951,10 @@ static PlanNode* par_plan(thread_db* tdbb, CompilerScratch* csb) // we have hit a stream; parse the context number and access type - if (node_type == blr_retrieve) + jrd_rel* relation = nullptr; + jrd_prc* procedure = nullptr; + + if (blrOp == blr_retrieve) { PlanNode* plan = FB_NEW_POOL(csb->csb_pool) PlanNode(csb->csb_pool, PlanNode::TYPE_RETRIEVE); @@ -959,39 +962,75 @@ static PlanNode* par_plan(thread_db* tdbb, CompilerScratch* csb) // itself is redundant except in the case of a view, // in which case the base relation (and alias) must be specified - USHORT n = (unsigned int) csb->csb_blr_reader.getByte(); - //// TODO: LocalTableSourceNode (blr_local_table_id) - if (n != blr_relation && n != blr_relation2 && n != blr_rid && n != blr_rid2) - PAR_syntax_error(csb, "TABLE"); + blrOp = csb->csb_blr_reader.getByte(); - // don't make RelationSourceNode::parse() parse the context, because - // this would add a new context; while this is a reference to + // Don't make RecordSourceNode::parse() to parse the context, because + // this would add a new context, while this is a reference to // an existing context - //// TODO: LocalTableSourceNode - plan->relationNode = RelationSourceNode::parse(tdbb, csb, n, false); + switch (blrOp) + { + case blr_relation: + case blr_rid: + case blr_relation2: + case blr_rid2: + { + const auto relationNode = RelationSourceNode::parse(tdbb, csb, blrOp, false); + plan->recordSourceNode = relationNode; + relation = relationNode->relation; + } + break; - jrd_rel* relation = plan->relationNode->relation; + case blr_pid: + case blr_pid2: + case blr_procedure: + case blr_procedure2: + case blr_procedure3: + case blr_procedure4: + case blr_subproc: + { + const auto procedureNode = ProcedureSourceNode::parse(tdbb, csb, blrOp, false); + plan->recordSourceNode = procedureNode; + procedure = procedureNode->procedure; + } + break; + + case blr_local_table_id: + // TODO + + default: + PAR_syntax_error(csb, "TABLE or PROCEDURE"); + } // CVC: bottleneck - n = csb->csb_blr_reader.getByte(); - if (n >= csb->csb_rpt.getCount() || !(csb->csb_rpt[n].csb_flags & csb_used)) + const auto context = csb->csb_blr_reader.getByte(); + if (context >= csb->csb_rpt.getCount() || !(csb->csb_rpt[context].csb_flags & csb_used)) PAR_error(csb, Arg::Gds(isc_ctxnotdef)); - const StreamType stream = csb->csb_rpt[n].csb_stream; + const StreamType stream = csb->csb_rpt[context].csb_stream; - plan->relationNode->setStream(stream); - plan->relationNode->context = n; + plan->recordSourceNode->setStream(stream); +// plan->recordSourceNode->context = context; not needed ??? + + if (procedure) + { + // Skip input parameters count, it's always zero for plans + const auto count = csb->csb_blr_reader.getWord(); + fb_assert(!count); + } // Access plan types (sequential is default) - node_type = (USHORT) csb->csb_blr_reader.getByte(); + blrOp = csb->csb_blr_reader.getByte(); const bool isGbak = tdbb->getAttachment()->isGbak(); - switch (node_type) + switch (blrOp) { case blr_navigational: { + if (procedure) + PAR_error(csb, Arg::Gds(isc_wrong_proc_plan)); + plan->accessType = FB_NEW_POOL(csb->csb_pool) PlanNode::AccessType(csb->csb_pool, PlanNode::AccessType::TYPE_NAVIGATIONAL); @@ -1049,6 +1088,9 @@ static PlanNode* par_plan(thread_db* tdbb, CompilerScratch* csb) } case blr_indices: { + if (procedure) + PAR_error(csb, Arg::Gds(isc_wrong_proc_plan)); + if (plan->accessType) csb->csb_blr_reader.getByte(); // skip blr_indices else @@ -1238,7 +1280,7 @@ RecordSourceNode* PAR_parseRecordSource(thread_db* tdbb, CompilerScratch* csb) case blr_procedure3: case blr_procedure4: case blr_subproc: - return ProcedureSourceNode::parse(tdbb, csb, blrOp); + return ProcedureSourceNode::parse(tdbb, csb, blrOp, true); case blr_rse: case blr_lateral_rse: