maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #08282
Re: [Commits] Rev 4382: MDEV-6892: WHERE does not apply in file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6892/
On 04.03.15 21:45, Sergey Petrunia wrote:
Hi Sanja,
Please find my feedback below. We should discuss it.
On Fri, Dec 19, 2014 at 01:24:12PM +0000, sanja@xxxxxxxxxxxx wrote:
At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6892/
------------------------------------------------------------
revno: 4382
revision-id: sanja@xxxxxxxxxxxx-20141219132350-6bo1g7q6gbivqdky
parent: svoj@xxxxxxxxxxx-20141201105829-ctaoe194abtorhz2
committer: sanja@xxxxxxxxxxxx
branch nick: work-maria-5.5-MDEV-6892
timestamp: Fri 2014-12-19 14:23:50 +0100
message:
MDEV-6892: WHERE does not apply
Taking into account implicit dependence of constant view field from nullable table of left join added.
Fixed finding real table to check if ut turned to NULL (materialized view & derived taken into account)
...
=== modified file 'sql/item.cc'
--- a/sql/item.cc 2014-10-06 17:53:55 +0000
+++ b/sql/item.cc 2014-12-19 13:23:50 +0000
@@ -9790,12 +9790,36 @@ void Item_ref::update_used_tables()
(*ref)->update_used_tables();
}
+void Item_direct_view_ref::update_used_tables()
+{
+ if (view->is_inner_table_of_outer_join())
+ {
+ null_ref_table= NULL;
+ check_null_ref();
update_used_tables() is called during optimization.
Why does it call check_null_ref() which may check null_ref_table->null_row?
Is check_null_ref() function an execution-time function (and so it computes
"current" value of an item)
or is it an optimization-time function (and so it computes static attributes
and can be called at any point in the query)
It looks like it's a bit of both, which is really confusing. Could you please
clarify?
can you clarify such devision of functions (I hear first time that we
have static atributes during optimisation (where we transform subqueries
and many other things)
check_null_ref() was introduced by you in another views+outer joins bugfix. Can
we get it documented?
yes, but not in this bugfix.
+ }
+ Item_direct_ref::update_used_tables();
+}
Overall, I don't understand why some checks are in used_tables() and some are
in update_used_tables(). Could you clarify?
used_tables declared static, so I have 2 posibilities:
1) remove static everywhere
2) repeat some code of check_null_ref() without changing class variables
I chose second
+
table_map Item_direct_view_ref::used_tables() const
{
+ TABLE *null_ref= null_ref_table;
+
+ if (view->is_inner_table_of_outer_join())
+ {
+ if (null_ref == NULL)
+ null_ref= view->get_real_join_table();
+ else if (null_ref == NO_NULL_TABLE)
+ null_ref= NULL;
+ }
+ else
+ null_ref= NULL;
+
return get_depended_from() ?
OUTER_REF_TABLE_BIT :
((view->is_merged_derived() || view->merged || !view->table) ?
- (*ref)->used_tables() :
+ ((*ref)->used_tables() ?
+ (*ref)->used_tables() :
+ (null_ref ? null_ref->map : (table_map)0 )) :
view->table->map);
Two-level nesting of conditional expressions is too complicated. Please change
into 'if' statement and a comment about what's going on.
OK
How can Item_direct_view_ref() deped on an outer table??
probably if view used in a subquery?
}
=== modified file 'sql/item.h'
--- a/sql/item.h 2014-11-18 14:42:40 +0000
+++ b/sql/item.h 2014-12-19 13:23:50 +0000
@@ -3319,7 +3319,8 @@ class Item_direct_view_ref :public Item_
{
if (null_ref_table == NULL)
{
- if (!(null_ref_table= view->get_real_join_table()))
+ if (!view->is_inner_table_of_outer_join() ||
+ !(null_ref_table= view->get_real_join_table()))
null_ref_table= NO_NULL_TABLE;
}
if (null_ref_table != NO_NULL_TABLE && null_ref_table->null_row)
@@ -3329,6 +3330,7 @@ class Item_direct_view_ref :public Item_
}
return FALSE;
}
+
public:
Item_direct_view_ref(Name_resolution_context *context_arg, Item **item,
const char *table_name_arg,
@@ -3353,8 +3355,10 @@ public:
bool subst_argument_checker(uchar **arg);
Item *equal_fields_propagator(uchar *arg);
Item *replace_equal_field(uchar *arg);
- table_map used_tables() const;
+ table_map used_tables() const;
+ void update_used_tables();
table_map not_null_tables() const;
+ bool const_item() const { return used_tables() == 0; }
bool walk(Item_processor processor, bool walk_subquery, uchar *arg)
{
return (*ref)->walk(processor, walk_subquery, arg) ||
=== modified file 'sql/table.cc'
--- a/sql/table.cc 2014-10-29 10:22:48 +0000
+++ b/sql/table.cc 2014-12-19 13:23:50 +0000
The function get_real_join_table() lacks comments, which makes it difficult to
review. Do I understand correctly that its action can be described by this
comment:
/*
@brief
Given a merged view (or a merged derived table), find its first* base** table.
@detail
(*) first means as defined in the syntax
(**) base means we need a physical table, one that has a bit in table->map
and is present in select_lex->leaf_tables.
Example:
t1 join
(
(select * from t2, t3 ) as derivedA
join
(select * from t5, t6) as derivedC
) as derivedB
here, derivedB->get_real_join_table()="t2"
*/
yes, but actually i'd replaced 'first' with 'any'
@@ -5012,7 +5012,8 @@ TABLE *TABLE_LIST::get_real_join_table()
TABLE_LIST *tbl= this;
while (tbl->table == NULL || tbl->table->reginfo.join_tab == NULL)
I'm quite surprised that joins are run over tables that don't have
reginfo.join_tab set appropriately...
{
- if (tbl->view == NULL && tbl->derived == NULL)
+ if ((tbl->view == NULL && tbl->derived == NULL) ||
+ tbl->is_materialized_derived())
I'm wondering why part of conditions are in while(...) and the other part is
here.
1) historically 2) readable
break;
/* we do not support merging of union yet */
DBUG_ASSERT(tbl->view == NULL ||
The code below this line looks bizarre (and it is still from your recent fixes
to VIEWs):
{
List_iterator_fast<TABLE_LIST> ti;
{
List_iterator_fast<TABLE_LIST>
ti(tbl->view != NULL ?
why define 'ti' and then immediately redefine it?
obviously it is an mistake (first definition is not needed)
BR
Sergei
References
