New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MDEV-28509 Dereferenced null pointer of type 'struct JOIN_TAB' in add_key_field #2821
base: 10.5
Are you sure you want to change the base?
Conversation
|
acd5099
to
a5bda76
Compare
a5bda76
to
e0517ee
Compare
e03e3d2
to
d1dfc95
Compare
d1dfc95
to
7cefbe5
Compare
sql/sql_select.cc
Outdated
_work(); | ||
} | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, don't do that. You can use SCOPE_EXIT
from include/scope.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed scope_exit
, thank you; it does exactly what I need 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uppercase. Like
SCOPE_EXIT([order] () {
...
});
7cefbe5
to
552a005
Compare
e8bb18d
to
a4a4699
Compare
…_key_field This patch fixes a crash when calculating join statistics during query optimization for queries with dangling WINDOW references. Put another way, the system may crash when a query defines a WINDOW but doesn't then refer to it. Item::marker is overloaded for different uses, the most typical refer to it as a bit field. However, the setup_group function uses it to mark that a field was found when traversing a GROUP BY. Originally, this marking set the Item::marker field to 1 to indicate that it was found. Later on in setup_group (and only when SQL mode ONLY_FULL_GROUP_BY is enabled), we would skip any such marked fields when checking that fields only referenced those found in the GROUP BY; otherwise, it would be silly to find fields of the GROUP BY within the GROUP BY field itself. Setting Item::marker to 1 seemed mostly harmless at that point in time. But later, in git sha 4d143a6, we introduced several changes: (1) the value of marker in setup_group was changed from 1 to UNDEF_POS, (2) Item::marker was changed from uint8 to int8 (which has since been changed to an int), and (3) UNDEF_POS which is defined to be -1 was also added. Queries that define WINDOWs internally will setup groups and orders as part of query processing via the setup_group function. Consequently because of the behavior described earlier above, such queries may have items with markers as UNDEF_POS (-1, or 0xffffffff) which is the same as marking all of the flag bits as set. This is disastrous for those users of Item::marker which refer to it as a bit field as some of those bits are mutually exclusive in meaning, and in many places we don't mask the bits we're interested in, we just compare the value of the field as a whole to some flag value with direct comparison. In particular, the method Item_direct_view_ref::grouping_field_transformer_for_where would incorrectly see that the ref's marker was set for substitution when it was actually -1, all bits set, taking the wrong execution path leading to the crash. Upon return from the setup_group function, we set the value of the marker flag to zero if we set it to -1. This preserves the marker behavior for 'full group by' (if configured) while not otherwise allowing the marker flag state to leak outside of this function.
a4a4699
to
777ba7f
Compare
This patch fixes a crash when calculating join statistics during query
optimization for queries with dangling WINDOW references. Put another way,
the system may crash when a query defines a WINDOW but doesn't then refer to
it.
Item::marker is overloaded for different uses, the most typical refer to it as
a bit field. However, the setup_group function uses it to mark that a field
was found when traversing a GROUP BY. Originally, this marking set the
Item::marker field to 1 to indicate that it was found. Later on in setup_group
(and only when SQL mode ONLY_FULL_GROUP_BY is enabled), we would skip any such
marked fields when checking that fields only referenced those found in the
GROUP BY; otherwise, it would be silly to find fields of the GROUP BY within
the GROUP BY field itself. Setting Item::marker to 1 seemed mostly harmless
at that point in time. But later, in git sha 4d143a6, we introduced
several changes: (1) the value of marker in setup_group was changed from 1 to
UNDEF_POS, (2) Item::marker was changed from uint8 to int8 (which has since
been changed to an int), and (3) UNDEF_POS which is defined to be -1 was also
added.
Queries that define WINDOWs internally will setup groups and orders as part
of query processing via the setup_group function. Consequently because of
the behavior described earlier above, such queries may have items with markers
as UNDEF_POS (-1, or 0xffffffff) which is the same as marking all of the flag
bits as set. This is disastrous for those users of Item::marker which refer
to it as a bit field as some of those bits are mutually exclusive in meaning,
and in many places we don't mask the bits we're interested in, we just
compare the value of the field as a whole to some flag value with direct
comparison. In particular, the method
Item_direct_view_ref::grouping_field_transformer_for_where would
incorrectly see that the ref's marker was set for substitution when it was
actually -1, all bits set, taking the wrong execution path leading to the
crash.
Upon return from the setup_group function, we set the value of the marker
flag to zero if we set it to -1. This preserves the marker behavior for
'full group by' (if configured) while not otherwise allowing the marker
flag state to leak outside of this function.