firebird/firebird-mirror
firebird/firebird-mirror
8
0
Fork 1
mirror of https://github.com/FirebirdSQL/firebird.git synced 2025-01-24 06:03:02 +01:00
Code Issues

Unfortunately, the only correct way to reference stream internals (e.g. get field descriptors) in node comparison routines is either to compare nodes along with their corresponding CSBs or to compare only nodes and handle streams outside. I'm favoring the latter approach. And no, remapping is a wrong solution either (trunk is to be reworked again in this regard, sigh). This fixes CORE-4139 at the cost of removed CAST vs FIELD comparison trick. Let's see whether somebody is going to be affected.

Browse Source
This commit is contained in:
dimitr 2013-07-07 15:11:06 +00:00
parent 0e7302f7e9
commit 123415bc5e
3 changed files with 62 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

167
src/jrd/Optimizer.cpp
View File

@ -363,9 +363,7 @@ void OPT_compute_rse_streams(const RecordSelExpr* rse, UCHAR* streams)
} }
bool OPT_expression_equal(thread_db* tdbb, OptimizerBlk* opt, bool OPT_expression_equal(const index_desc* idx, jrd_nod* node, USHORT stream)
const index_desc* idx, jrd_nod* node,
USHORT stream)
{ {
/************************************** /**************************************
* *
@ -379,55 +377,29 @@ bool OPT_expression_equal(thread_db* tdbb, OptimizerBlk* opt,
**************************************/ **************************************/
DEV_BLKCHK(node, type_nod); DEV_BLKCHK(node, type_nod);
SET_TDBB(tdbb); if (idx && idx->idx_expression)
if (idx && idx->idx_expression_request && idx->idx_expression)
{ {
fb_assert(idx->idx_flags & idx_expressn); fb_assert(idx->idx_flags & idx_expressn);
jrd_req* org_request = tdbb->getRequest(); if (OPT_expression_equal2(idx->idx_expression, node, true))
jrd_req* expr_request = EXE_find_request(tdbb, idx->idx_expression_request, false);
fb_assert(expr_request->req_caller == NULL);
expr_request->req_caller = org_request;
tdbb->setRequest(expr_request);
bool result = false;
try
{ {
Jrd::ContextPoolHolder context(tdbb, expr_request->req_pool); SortedStreamList expr_streams, node_streams;
OPT_get_expression_streams(idx->idx_expression, expr_streams);
OPT_get_expression_streams(node, node_streams);
expr_request->req_timestamp = expr_request->req_caller ? if (expr_streams.getCount() == 1 && expr_streams[0] == 0 &&
expr_request->req_caller->req_timestamp : Firebird::TimeStamp::getCurrentTimeStamp(); node_streams.getCount() == 1 && node_streams[0] == stream)
{
result = OPT_expression_equal2(tdbb, opt, idx->idx_expression, node, stream); return true;
}
} }
catch (const Firebird::Exception&)
{
tdbb->setRequest(org_request);
expr_request->req_caller = NULL;
expr_request->req_flags &= ~req_in_use;
expr_request->req_timestamp.invalidate();
throw;
}
tdbb->setRequest(org_request);
expr_request->req_caller = NULL;
expr_request->req_flags &= ~req_in_use;
expr_request->req_timestamp.invalidate();
return result;
} }
return false; return false;
} }
bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt, bool OPT_expression_equal2(jrd_nod* node1, jrd_nod* node2, bool ignoreStreams)
jrd_nod* node1, jrd_nod* node2,
USHORT stream)
{ {
/************************************** /**************************************
* *
@ -444,8 +416,6 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
DEV_BLKCHK(node1, type_nod); DEV_BLKCHK(node1, type_nod);
DEV_BLKCHK(node2, type_nod); DEV_BLKCHK(node2, type_nod);
SET_TDBB(tdbb);
if (!node1 || !node2) if (!node1 || !node2)
{ {
BUGCHECK(303); // msg 303 Invalid expression for evaluation. BUGCHECK(303); // msg 303 Invalid expression for evaluation.
@ -453,42 +423,11 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
if (node1->nod_type != node2->nod_type) if (node1->nod_type != node2->nod_type)
{ {
dsc dsc1, dsc2;
dsc *desc1 = &dsc1, *desc2 = &dsc2;
if (node1->nod_type == nod_cast)
{
jrd_nod* const source = node1->nod_arg[e_cast_source];
CMP_get_desc(tdbb, opt->opt_csb, node1, desc1);
CMP_get_desc(tdbb, opt->opt_csb, source, desc2);
if (DSC_EQUIV(desc1, desc2, true) &&
OPT_expression_equal2(tdbb, opt, source, node2, stream))
{
return true;
}
}
if (node2->nod_type == nod_cast)
{
jrd_nod* const source = node2->nod_arg[e_cast_source];
CMP_get_desc(tdbb, opt->opt_csb, node2, desc1);
CMP_get_desc(tdbb, opt->opt_csb, source, desc2);
if (DSC_EQUIV(desc1, desc2, true) &&
OPT_expression_equal2(tdbb, opt, source, node1, stream))
{
return true;
}
}
if (node1->nod_type == nod_derived_expr) if (node1->nod_type == nod_derived_expr)
{ {
jrd_nod* const expression = node1->nod_arg[e_derived_expr_expr]; jrd_nod* const expression = node1->nod_arg[e_derived_expr_expr];
if (OPT_expression_equal2(tdbb, opt, expression, node2, stream)) if (OPT_expression_equal2(expression, node2, ignoreStreams))
{ {
return true; return true;
} }
@ -498,7 +437,7 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
{ {
jrd_nod* const expression = node2->nod_arg[e_derived_expr_expr]; jrd_nod* const expression = node2->nod_arg[e_derived_expr_expr];
if (OPT_expression_equal2(tdbb, opt, expression, node1, stream)) if (OPT_expression_equal2(expression, node1, ignoreStreams))
{ {
return true; return true;
} }
@ -521,8 +460,8 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
// A+B is equivalent to B+A, ditto A*B==B*A // A+B is equivalent to B+A, ditto A*B==B*A
// Note: If one expression is A+B+C, but the other is B+C+A we won't // Note: If one expression is A+B+C, but the other is B+C+A we won't
// necessarily match them. // necessarily match them.
if (OPT_expression_equal2(tdbb, opt, node1->nod_arg[0], node2->nod_arg[1], stream) && if (OPT_expression_equal2(node1->nod_arg[0], node2->nod_arg[1], ignoreStreams) &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[1], node2->nod_arg[0], stream)) OPT_expression_equal2(node1->nod_arg[1], node2->nod_arg[0], ignoreStreams))
{ {
return true; return true;
} }
@ -538,8 +477,8 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_geq: case nod_geq:
case nod_leq: case nod_leq:
case nod_lss: case nod_lss:
if (OPT_expression_equal2(tdbb, opt, node1->nod_arg[0], node2->nod_arg[0], stream) && if (OPT_expression_equal2(node1->nod_arg[0], node2->nod_arg[0], ignoreStreams) &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[1], node2->nod_arg[1], stream)) OPT_expression_equal2(node1->nod_arg[1], node2->nod_arg[1], ignoreStreams))
{ {
return true; return true;
} }
@ -547,27 +486,25 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_rec_version: case nod_rec_version:
case nod_dbkey: case nod_dbkey:
if (node1->nod_arg[0] == node2->nod_arg[0]) if (ignoreStreams || node1->nod_arg[0] == node2->nod_arg[0])
{ {
return true; return true;
} }
break; break;
case nod_field: case nod_field:
if ((node1->nod_arg[e_fld_id] == node2->nod_arg[e_fld_id]) &&
(ignoreStreams || (node1->nod_arg[e_fld_stream] == node2->nod_arg[e_fld_stream])))
{ {
const USHORT fld_stream = (USHORT)(IPTR) node2->nod_arg[e_fld_stream]; return true;
if ((node1->nod_arg[e_fld_id] == node2->nod_arg[e_fld_id]) && fld_stream == stream)
{
return true;
}
} }
break; break;
case nod_function: case nod_function:
if (node1->nod_arg[e_fun_function] && if (node1->nod_arg[e_fun_function] &&
(node1->nod_arg[e_fun_function] == node2->nod_arg[e_fun_function]) && (node1->nod_arg[e_fun_function] == node2->nod_arg[e_fun_function]) &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[e_fun_args], OPT_expression_equal2(node1->nod_arg[e_fun_args],
node2->nod_arg[e_fun_args], stream)) node2->nod_arg[e_fun_args], ignoreStreams))
{ {
return true; return true;
} }
@ -576,8 +513,8 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_sys_function: case nod_sys_function:
if (node1->nod_arg[e_sysfun_function] && if (node1->nod_arg[e_sysfun_function] &&
(node1->nod_arg[e_sysfun_function] == node2->nod_arg[e_sysfun_function]) && (node1->nod_arg[e_sysfun_function] == node2->nod_arg[e_sysfun_function]) &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[e_sysfun_args], OPT_expression_equal2(node1->nod_arg[e_sysfun_args],
node2->nod_arg[e_sysfun_args], stream)) node2->nod_arg[e_sysfun_args], ignoreStreams))
{ {
return true; return true;
} }
@ -585,9 +522,13 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_literal: case nod_literal:
{ {
const dsc* desc1 = EVL_expr(tdbb, node1); const dsc* const desc1 = &((Literal*) node1)->lit_desc;
const dsc* desc2 = EVL_expr(tdbb, node2); const UCHAR* const ptr1 = desc1->dsc_address;
if (desc1 && desc2 && !MOV_compare(desc1, desc2))
const dsc* const desc2 = &((Literal*) node2)->lit_desc;
const UCHAR* const ptr2 = desc2->dsc_address;
if (DSC_EQUIV(desc1, desc2, true) && !memcmp(ptr1, ptr2, desc1->dsc_length))
{ {
return true; return true;
} }
@ -622,7 +563,7 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
} }
for (int i = 0; i < node1->nod_count; ++i) for (int i = 0; i < node1->nod_count; ++i)
{ {
if (!OPT_expression_equal2(tdbb, opt, node1->nod_arg[i], node2->nod_arg[i], stream)) if (!OPT_expression_equal2(node1->nod_arg[i], node2->nod_arg[i], ignoreStreams))
{ {
return false; return false;
} }
@ -641,7 +582,7 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_negate: case nod_negate:
case nod_internal_info: case nod_internal_info:
if (OPT_expression_equal2(tdbb, opt, node1->nod_arg[0], node2->nod_arg[0], stream)) if (OPT_expression_equal2(node1->nod_arg[0], node2->nod_arg[0], ignoreStreams))
{ {
return true; return true;
} }
@ -649,7 +590,7 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_upcase: case nod_upcase:
case nod_lowcase: case nod_lowcase:
if (OPT_expression_equal2(tdbb, opt, node1->nod_arg[0], node2->nod_arg[0], stream)) if (OPT_expression_equal2(node1->nod_arg[0], node2->nod_arg[0], ignoreStreams))
{ {
return true; return true;
} }
@ -657,15 +598,15 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_cast: case nod_cast:
{ {
dsc dsc1, dsc2; const Format* const format1 = (Format*) node1->nod_arg[e_cast_fmt];
dsc *desc1 = &dsc1, *desc2 = &dsc2; const dsc* const desc1 = &format1->fmt_desc[0];
CMP_get_desc(tdbb, opt->opt_csb, node1, desc1); const Format* const format2 = (Format*) node2->nod_arg[e_cast_fmt];
CMP_get_desc(tdbb, opt->opt_csb, node2, desc2); const dsc* const desc2 = &format2->fmt_desc[0];
if (DSC_EQUIV(desc1, desc2, true) && if (DSC_EQUIV(desc1, desc2, true) &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[e_cast_source], OPT_expression_equal2(node1->nod_arg[e_cast_source],
node2->nod_arg[e_cast_source], stream)) node2->nod_arg[e_cast_source], ignoreStreams))
{ {
return true; return true;
} }
@ -674,8 +615,8 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_extract: case nod_extract:
if (node1->nod_arg[e_extract_part] == node2->nod_arg[e_extract_part] && if (node1->nod_arg[e_extract_part] == node2->nod_arg[e_extract_part] &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[e_extract_value], OPT_expression_equal2(node1->nod_arg[e_extract_value],
node2->nod_arg[e_extract_value], stream)) node2->nod_arg[e_extract_value], ignoreStreams))
{ {
return true; return true;
} }
@ -683,16 +624,16 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
case nod_strlen: case nod_strlen:
if (node1->nod_arg[e_strlen_type] == node2->nod_arg[e_strlen_type] && if (node1->nod_arg[e_strlen_type] == node2->nod_arg[e_strlen_type] &&
OPT_expression_equal2(tdbb, opt, node1->nod_arg[e_strlen_value], OPT_expression_equal2(node1->nod_arg[e_strlen_value],
node2->nod_arg[e_strlen_value], stream)) node2->nod_arg[e_strlen_value], ignoreStreams))
{ {
return true; return true;
} }
break; break;
case nod_derived_expr: case nod_derived_expr:
if (OPT_expression_equal2(tdbb, opt, node1->nod_arg[e_derived_expr_expr], if (OPT_expression_equal2(node1->nod_arg[e_derived_expr_expr],
node2->nod_arg[e_derived_expr_expr], stream)) node2->nod_arg[e_derived_expr_expr], ignoreStreams))
{ {
return true; return true;
} }
@ -712,7 +653,7 @@ bool OPT_expression_equal2(thread_db* tdbb, OptimizerBlk* opt,
while (count--) while (count--)
{ {
if (!OPT_expression_equal2(tdbb, opt, *ptr1++, *ptr2++, stream)) if (!OPT_expression_equal2(*ptr1++, *ptr2++, ignoreStreams))
{ {
return false; return false;
} }
@ -1705,7 +1646,7 @@ RecordSource* OptimizerRetrieval::generateNavigation()
jrd_nod* node = *ptr; jrd_nod* node = *ptr;
if (idx->idx_flags & idx_expressn) if (idx->idx_flags & idx_expressn)
{ {
if (!OPT_expression_equal(tdbb, optimizer, idx, node, stream)) if (!OPT_expression_equal(idx, node, stream))
{ {
usableIndex = false; usableIndex = false;
break; break;
@ -2627,11 +2568,11 @@ bool OptimizerRetrieval::matchBoolean(IndexScratch* indexScratch, jrd_nod* boole
fb_assert(indexScratch->idx->idx_expression != NULL); fb_assert(indexScratch->idx->idx_expression != NULL);
if (!OPT_expression_equal(tdbb, optimizer, indexScratch->idx, match, stream) || if (!OPT_expression_equal(indexScratch->idx, match, stream) ||
(value && !OPT_computable(optimizer->opt_csb, value, stream, true, false))) (value && !OPT_computable(optimizer->opt_csb, value, stream, true, false)))
{ {
if (boolean->nod_type != nod_starts && value && if (boolean->nod_type != nod_starts && value &&
OPT_expression_equal(tdbb, optimizer, indexScratch->idx, value, stream) && OPT_expression_equal(indexScratch->idx, value, stream) &&
OPT_computable(optimizer->opt_csb, match, stream, true, false)) OPT_computable(optimizer->opt_csb, match, stream, true, false))
{ {
match = boolean->nod_arg[1]; match = boolean->nod_arg[1];
@ -3207,13 +3148,13 @@ bool OptimizerRetrieval::validateStarts(IndexScratch* indexScratch,
// AB: What if the expression contains a number/float etc.. and // AB: What if the expression contains a number/float etc.. and
// we use starting with against it? Is that allowed? // we use starting with against it? Is that allowed?
fb_assert(indexScratch->idx->idx_expression != NULL); fb_assert(indexScratch->idx->idx_expression != NULL);
if (!(OPT_expression_equal(tdbb, optimizer, indexScratch->idx, field, stream) || if (!(OPT_expression_equal(indexScratch->idx, field, stream) ||
(value && !OPT_computable(optimizer->opt_csb, value, stream, true, false)))) (value && !OPT_computable(optimizer->opt_csb, value, stream, true, false))))
{ {
// AB: Can we swap de left and right sides by a starting with? // AB: Can we swap de left and right sides by a starting with?
// X STARTING WITH 'a' that is never the same as 'a' STARTING WITH X // X STARTING WITH 'a' that is never the same as 'a' STARTING WITH X
if (value && if (value &&
OPT_expression_equal(tdbb, optimizer, indexScratch->idx, value, stream) && OPT_expression_equal(indexScratch->idx, value, stream) &&
OPT_computable(optimizer->opt_csb, field, stream, true, false)) OPT_computable(optimizer->opt_csb, field, stream, true, false))
{ {
field = value; field = value;

4
src/jrd/Optimizer.h
View File

@ -73,8 +73,8 @@ class jrd_rel;
bool OPT_computable(CompilerScratch*, const jrd_nod*, SSHORT, const bool, const bool); bool OPT_computable(CompilerScratch*, const jrd_nod*, SSHORT, const bool, const bool);
void OPT_compute_rse_streams(const RecordSelExpr*, UCHAR*); void OPT_compute_rse_streams(const RecordSelExpr*, UCHAR*);
bool OPT_expression_equal(thread_db*, OptimizerBlk*, const index_desc*, jrd_nod*, USHORT); bool OPT_expression_equal(const index_desc*, jrd_nod*, USHORT);
bool OPT_expression_equal2(thread_db*, OptimizerBlk*, jrd_nod*, jrd_nod*, USHORT); bool OPT_expression_equal2(jrd_nod*, jrd_nod*, bool);
jrd_nod* OPT_find_dbkey(jrd_nod* dbkey, USHORT stream, SLONG* position); jrd_nod* OPT_find_dbkey(jrd_nod* dbkey, USHORT stream, SLONG* position);
void OPT_get_expression_streams(const jrd_nod*, Firebird::SortedArray<int>&); void OPT_get_expression_streams(const jrd_nod*, Firebird::SortedArray<int>&);
double OPT_getRelationCardinality(thread_db*, jrd_rel*, const Format*); double OPT_getRelationCardinality(thread_db*, jrd_rel*, const Format*);

12
src/jrd/opt.cpp
View File

@ -4568,7 +4568,7 @@ static RecordSource* gen_navigation(thread_db* tdbb,
jrd_nod* node = *ptr; jrd_nod* node = *ptr;
if (idx->idx_flags & idx_expressn) if (idx->idx_flags & idx_expressn)
{ {
if (!OPT_expression_equal(tdbb, opt, idx, node, stream)) if (!OPT_expression_equal(idx, node, stream))
return NULL; return NULL;
} }
else if (node->nod_type != nod_field || else if (node->nod_type != nod_field ||
@ -6674,7 +6674,7 @@ static jrd_nod* make_missing(thread_db* tdbb,
if (idx->idx_flags & idx_expressn) if (idx->idx_flags & idx_expressn)
{ {
fb_assert(idx->idx_expression != NULL); fb_assert(idx->idx_expression != NULL);
if (!OPT_expression_equal(tdbb, opt, idx, field, stream)) if (!OPT_expression_equal(idx, field, stream))
{ {
return NULL; return NULL;
} }
@ -6757,10 +6757,10 @@ static jrd_nod* make_starts(thread_db* tdbb,
if (idx->idx_flags & idx_expressn) if (idx->idx_flags & idx_expressn)
{ {
fb_assert(idx->idx_expression != NULL); fb_assert(idx->idx_expression != NULL);
if (!(OPT_expression_equal(tdbb, opt, idx, field, stream) && if (!(OPT_expression_equal(idx, field, stream) &&
OPT_computable(opt->opt_csb, value, stream, true, false))) OPT_computable(opt->opt_csb, value, stream, true, false)))
{ {
if (OPT_expression_equal(tdbb, opt, idx, value, stream) && if (OPT_expression_equal(idx, value, stream) &&
OPT_computable(opt->opt_csb, field, stream, true, false)) OPT_computable(opt->opt_csb, field, stream, true, false))
{ {
field = value; field = value;
@ -7078,10 +7078,10 @@ static int match_index(thread_db* tdbb, OptimizerBlk* opt, SSHORT stream, jrd_no
fb_assert(idx->idx_expression != NULL); fb_assert(idx->idx_expression != NULL);
if (!OPT_expression_equal(tdbb, opt, idx, match, stream) || if (!OPT_expression_equal(idx, match, stream) ||
(value && !OPT_computable(opt->opt_csb, value, stream, true, false))) (value && !OPT_computable(opt->opt_csb, value, stream, true, false)))
{ {
if (value && OPT_expression_equal(tdbb, opt, idx, value, stream) && if (value && OPT_expression_equal(idx, value, stream) &&
OPT_computable(opt->opt_csb, match, stream, true, false)) OPT_computable(opt->opt_csb, match, stream, true, false))
{ {
match = boolean->nod_arg[1]; match = boolean->nod_arg[1];