cockroachdb / cockroach

CockroachDB - the open source, cloud-native distributed SQL database.
https://www.cockroachlabs.com
Other
29.51k stars 3.7k forks source link

opt: bad index selected in single table query #38017

Closed tim-o closed 3 years ago

tim-o commented 5 years ago

This Github issue is synchronized with Zendesk:

Ticket ID: #3390 Group: Support Requester: lee.burgess@unibet.com Organization: Kindred Group Assignee: Ricardo Rocha Issue escalated by: Ricardo Rocha

Original ticket description:

tx_pte.txt contains the DDL and Statistics for the TX Table.

If I actually execute the query and force/hint it to use the index “tx@tx_idx_3” it runs in 200 ms.

Explain Plan

> tree | field | description
> +-----------------+--------+-----------------------------------------------------------+
> render | |
> └── index-join | |
> │ | table | tx@tx_pk
> └── scan | |
> | table | tx@tx_idx_3
> | spans | /"DEPOSIT_COMPLETED"/!NULL-/"DEPOSIT_COMPLETED"/PrefixEnd
> | filter | updated < (current_timestamp() - '30 days')
> (7 rows)
>
> Time: 2.608133ms

Real-time Execution:

select * FROM tx@tx@tx_idx_3 AS tx0_
WHERE tx0_.current_state
IN ('DEPOSIT_BLOCKED_BY_LIMIT',
'DEPOSIT_TRANSFER_RETRY',
'DEPOSIT_TRANSACTION_CREATED',
'DEPOSIT_INITIATED')
AND tx0_.updated < current_timestamp() - interval '30 days'
ORDER BY tx0_.updated ASC;

Time: 271.145094ms

Without hinting it.

Explain Plan

> tree | field | description
> +-----------------+--------+-----------------------------------------------------------+
> render | |
> └── sort | |
> │ | order | +updated
> └── scan | |
> | table | tx@tx_pk
> | spans | ALL
> | filter | (current_state IN ('DEPOSIT_BLOCKED_BY_LIMIT',
> 'DEPOSIT_INITIATED', 'DEPOSIT_TRANSACTION_CREATED',
> 'DEPOSIT_TRANSFER_RETRY')) AND (updated < (current_timestamp() - '30 days'))
> (7 rows)

Real-time Execution:

select * FROM tx AS tx0_
WHERE tx0_.current_state
IN ('DEPOSIT_BLOCKED_BY_LIMIT',
'DEPOSIT_TRANSFER_RETRY',
'DEPOSIT_TRANSACTION_CREATED',
'DEPOSIT_INITIATED')
AND tx0_.updated < current_timestamp() - interval '30 days'
ORDER BY tx0_.updated ASC;

Time: 4m39.544897691s

We have implemented the work around where we force tbl@index and execution is around the 200-300 ms, but why does the optimizer not choose the index as it is clearly faster.

justinj commented 5 years ago

cc @rytaft, this is the issue I was telling you about that I think is fundamentally a histogram issue, though I don't know for sure. @ricardocrdb was asking if we had a GitHub issue for them, I don't think we do, right?

rytaft commented 5 years ago

This first plan doesn't really make sense given the query -- the plan references "DEPOSIT_COMPLETED", while that value doesn't appear anywhere in the query. Are you sure this is the right plan?

If the index is on current_state, I would think we should be able to find the right plan and use that index. Since it looks like the customer is using 19.1.1, it would be great if they could rerun both queries (with and without hinting) using EXPLAIN (opt, env) <query>. Thanks!

rytaft commented 5 years ago

Regarding histograms, I don't believe we have an issue, although I'm happy to create one if it would be helpful.

RaduBerinde commented 5 years ago

I think that the problem here may be that the stats (in the ticket) show only 4 distinct values for current_state. So a filter like IN ('DEPOSIT_BLOCKED_BY_LIMIT', 'DEPOSIT_TRANSFER_RETRY', 'DEPOSIT_TRANSACTION_CREATED', 'DEPOSIT_INITIATED') is estimated to have very low selectivity. In reality, I'm guessing that some of these four values don't show up anywhere in the table. This would indeed be fixed by histograms because we would be able to detect that there is little overlap between the values.

rytaft commented 5 years ago

Ah good point -- didn't notice the attached file in the ticket. I agree that histograms are the long-term solution here, although I think there may be something we can do in the mean time. In particular, the fact that there are 4 distinct values before the filter and 4 after currently makes the stats code think that the filter is not selective at all (i.e., selectivity=1). I can try to tweak the code a bit to force filters such as this one to have selectivity < 1.

RaduBerinde commented 5 years ago

Note that in this case the good plan has an index join, so I'm guessing the selectivity value would need to be quite small.

rytaft commented 5 years ago

Good point - we won't be able to determine whether the index join is worth it without histograms. I just confirmed that we already do the right thing if the index is covering, so I don't think it's worth it to make the change I described above.

github-actions[bot] commented 3 years ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 5 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!

rytaft commented 3 years ago

Closing this since histograms have been available for several releases now, and based on previous comments we thought they would fix this issue. If this user is still seeing a bad plan on a newer release feel free to open another issue.