-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
opt: choosing suboptimal plan on TPCH query 6 after stats collected on lineitem
#46677
Comments
If you still have the setup, could you run the same experiment with vectorized off? Trying to ascertain whether inaccurate costing with vectorized plays a part. |
Sure. I reran the query on 3 node cluster on my laptop. Without stats
With stats
|
Thanks, so it looks like vectorized is responsible for a 3x improvement in the former case, and the optimizer doesn't know about that. But we're still 2x slower with stats though which is unfortunate. From our Indeed, I added the query with a
No index join:
I think the cost for a full primary scan is exaggerated here. I think the best course of action is to work on #46560 and then revisit and check if it fixes the problem. |
@yuzefovich Is this still an issue? I know you tested TPC-H recently and got good results thanks to join ordering and execution improvements. |
@rytaft this issue still appears to be present:
|
I did some more investigation here. It seems like our scan costing is actually not too bad. For the secondary index scan, we estimate a cost of 981416, while for the primary index scan, we estimate a cost of 7201462 (~7x larger). Based on the The real problem, however, is the costing of the index join. The total cost of the index join is 4879288, of which 981416 comes from the scan, leaving 3897872 for the join operation (about half the cost of the primary index scan). But the Part of the problem is that we're not costing index joins the same way we cost lookup joins. If I change the index join costing code to be closer to what we do for lookup joins, then the cost is larger: 6721750 instead of 4879288. Still, it's not quite enough to cause the optimizer to prefer the primary index scan. |
Prior to this commit, the cost of an index join was much less than the cost of an equivalent lookup join. Since index joins have the same implementation as lookup joins when the lookup columns form a key in the lookup table, the two operators should have the same cost. This commit updates the coster so that this is the case. Informs cockroachdb#46677 Release note (performance improvement): Updated the cost model in the optimizer to make index joins more expensive and better reflect the reality of their cost. As a result, the optimizer will choose index joins less frequently, generally resulting in more efficient query plans.
Prior to this commit, the cost of an index join was much less than the cost of an equivalent lookup join. Since index joins have the same implementation as lookup joins when the lookup columns form a key in the lookup table, the two operators should have the same cost. This commit updates the coster so that this is the case. Informs cockroachdb#46677 Release note (performance improvement): Updated the cost model in the optimizer to make index joins more expensive and better reflect the reality of their cost. As a result, the optimizer will choose index joins less frequently, generally resulting in more efficient query plans.
54768: opt: update cost of index join and statistics of array contains operation r=rytaft a=rytaft **opt: update statistics of array contains operation to match json contains** Prior to this commit, the selectivity of `j @> '{"a": "b"}'::json` was calculated as `1/9`, while the selectivity of `a @> array[1]` was calculated as `1/3`. Both of these predicates essentially represent one "path" that can be queried in an inverted index, so they should have the same selectivity. Furthermore, multiple paths should be more selective, so in the same way that the predicate `j @> '{"a": "b", "c": "d"}'::json` has selectivity `(1/9 * 1/9)`, `a @> array[1,2]` should similarly have selectivity `(1/9 * 1/9)`. This commit updates the logic in the statistics builder so that this is the case. Release note (performance improvement): Improved the selectivity estimate for array contains predicates (e.g., `arr @> ARRAY[1]`) in the optimizer. This improves the optimizer's cardinality estimation for queries containing these predicates, and may result in better query plans in some cases. **opt: update cost of index join to match cost of lookup join** Prior to this commit, the cost of an index join was much less than the cost of an equivalent lookup join. Since index joins have the same implementation as lookup joins when the lookup columns form a key in the lookup table, the two operators should have the same cost. This commit updates the coster so that this is the case. Informs #46677 Release note (performance improvement): Updated the cost model in the optimizer to make index joins more expensive and better reflect the reality of their cost. As a result, the optimizer will choose index joins less frequently, generally resulting in more efficient query plans. 54772: workload: bump the tpcc fixture version; create statistics when making fixtures r=RaduBerinde a=RaduBerinde #### workload: bump the tpcc fixture version The TPCC fixtures need refreshing: they contain FK indexes that we no longer need, and they are missing stats for some of the tables. This commit bumps the fixture version and incorporates the `deprecated-fk-indexes` flag into the fixture name. Release note: None #### workload: create statistics when making fixtures `workload make` imports a workload and then creates the backup (that can be later used by `workload load`). This change adds functionality to call CREATE STATISTICS on all tables before creating the backup. This can be turned off using a flag. Release note: None I am in the process of regenerating the fixtures with the new version. Informs #54702. Informs #50911. 54784: deps: bump cockroachdb/redact r=andreimatei a=knz To pick up the pointer formatting fix from cockroachdb/redact#9 Release note: None 54804: sql: skip TestSchemaChangeEvalContext r=yuzefovich a=yuzefovich Refs: #54775 Reason: flaky test Generated by bin/skip-test. Release justification: non-production code changes Release note: None Co-authored-by: Rebecca Taft <[email protected]> Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]>
Prior to this commit, the cost of an index join was much less than the cost of an equivalent lookup join. Since index joins have the same implementation as lookup joins when the lookup columns form a key in the lookup table, the two operators should have the same cost. This commit updates the coster so that this is the case. Informs cockroachdb#46677 Release note (performance improvement): Updated the cost model in the optimizer to make index joins more expensive and better reflect the reality of their cost. As a result, the optimizer will choose index joins less frequently, generally resulting in more efficient query plans.
This issue is still present. The optimizer is favouring index join over primary index scan, but index join ended up being more expensive than primary index scan. Here are the statement bundles: debug-primaryindexscan.zip and debug-indexjoin.zip. The Row estimation is not bad. It could be because the assigned cost for primary index scan is too high or the cost for index join is still not high enough. |
We have marked this issue as stale because it has been inactive for |
I've been digging into a possible performance regression with vectorized engine here and noticed that the query runtimes significantly increase after the collection of statistics on
lineitem
table. I confirmed that it is both present on master and 19.2.On a 3 node cluster on my laptop:
before stats are collected:
after stats are collected:
Plan before:
After:
This issue can be reproduced on a single node local cluster by restoring TPCH dataset for Scale factor of 1. Please let me know if any more info is needed.
Jira issue: CRDB-5069
The text was updated successfully, but these errors were encountered: