Re: sqlsmith crash incremental sort

On Thu, Apr 23, 2020 at 03:28:21PM +0800, Richard Guo wrote:
>On Thu, Apr 23, 2020 at 6:59 AM Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
>wrote:
>
>> I've pushed fix with the DEFAULT_NUM_DISTINCT. The input comes from a
>> set operation (which is where we call generate_append_tlist), so it's
>> probably fairly unique, so maybe we should use input_tuples. But it's
>> not guaranteed, so DEFAULT_NUM_DISTINCT seems reasonably defensive.
>>
>
>Thanks for the fix. Verified that the crash has been fixed.
>
>
>>
>> One detail I've changed is that instead of matching the expression
>> directly to a Var, it now calls pull_varnos() to also detect Vars
>> somewhere deeper. Lookig at examine_variable() it calls find_base_rel
>> for such case too, but I haven't tried constructing a query triggering
>> the issue.
>>
>
>A minor comment is that I don't think we need to strip relabel
>explicitly before calling pull_varnos(), because this function would
>recurse into T_RelabelType nodes.
>

Hmmm, yeah. I think you're right that's unnecessary. I misread the
walker function, I think.

>Also do we need to call bms_free(varnos) for each pathkey here to avoid
>waste of memory?
>

I don't think so. It wouldn't hurt, but we don't do that for other
pull_vernos calls either AFAICS.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services