Skip to content
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

global index: specify the table partition, but return all partitions' rows #40497

Closed
L-maple opened this issue Jan 11, 2023 · 7 comments · Fixed by #41197
Closed

global index: specify the table partition, but return all partitions' rows #40497

L-maple opened this issue Jan 11, 2023 · 7 comments · Fixed by #41197
Assignees
Labels
component/tablepartition This issue is related to Table Partition of TiDB. severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@L-maple
Copy link
Contributor

L-maple commented Jan 11, 2023

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

  drop table if exists p;
  create table p (id int, c int) partition by range (c) (
       partition p0 values less than (4),
       partition p1 values less than (7),
       partition p2 values less than (10));
  alter table p add unique idx(id);
  insert into p values (1,3), (3,4), (5,6), (7,9);
  select * from p partition(p0) use index (idx);

2. What did you expect to see? (Required)

when execturing the query: select * from p partition(p0) use index (idx)
the result should be: "1 3"

3. What did you see instead (Required)

[”1 3“, ”3 4“, ”5 6“, "7 9"]

4. What is your TiDB version? (Required)

version: release-5.0

+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version() |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v5.0.0-414-g6ca79c09c3-dirty
Edition: Community
Git Commit Hash: 6ca79c09c34ecbda1a49a961c049b477eb8f6a65
UTC Build Time: 2023-01-10 08:47:50
GoVersion: go1.16.10
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.77 sec)

@L-maple L-maple added the type/bug The issue is confirmed as a bug. label Jan 11, 2023
@L-maple
Copy link
Contributor Author

L-maple commented Jan 11, 2023

I want to contribute this issue, and I have two solutions:

  • When buildDataSource, remove the global index if the partitionName is specified.
  • When IndexScan is done, only send tableScan request to the specified partitions.

Which solution is better in your opinion? @tiancaiamao

@L-maple
Copy link
Contributor Author

L-maple commented Jan 11, 2023

I analyze the global index code logic, it seems global index would ignore partition table related logic in executorBuilder.buildIndexLookUpReader.
So I understand returning error directly is nice.

@jebter jebter added sig/sql-infra SIG: SQL Infra component/tablepartition This issue is related to Table Partition of TiDB. labels Jan 11, 2023
@tiancaiamao
Copy link
Contributor

  • When IndexScan is done, only send tableScan request to the specified partitions.

This is viable, let the index scan filter out the result belong to partition(p0)

@mjonss mjonss added feature/developing the related feature is in development and removed feature/developing the related feature is in development labels Jan 13, 2023
@mjonss mjonss changed the title global index: specify the table partition, but return all partitions' rows unique index: specify the table partition, but return all partitions' rows Jan 13, 2023
@L-maple
Copy link
Contributor Author

L-maple commented Jan 17, 2023

@tiancaiamao I pull the request, please review it.

@mjonss mjonss changed the title unique index: specify the table partition, but return all partitions' rows global index: specify the table partition, but return all partitions' rows Jan 17, 2023
@mjonss
Copy link
Contributor

mjonss commented Jan 17, 2023

In master aa51b46 it does not select the index access, but table scan and gives correct result:

tidb> explain select * from p partition(p0) use index (idx);
+-----------------------+---------+-----------+---------------+----------------------+
| id                    | estRows | task      | access object | operator info        |
+-----------------------+---------+-----------+---------------+----------------------+
| TableReader_5         | 4.00    | root      | partition:p0  | data:TableFullScan_4 |
| └─TableFullScan_4 | 4.00    | cop[tikv] | table:p       | keep order:false     |
+-----------------------+---------+-----------+---------------+----------------------+
2 rows in set (0.00 sec)

tidb> select * from p partition(p0) use index (idx);
+------+------+
| id   | c    |
+------+------+
|    1 |    3 |
+------+------+
1 row in set (0.00 sec)

Note that to test this one needs to enable global index (which is not yet supported). Either by config file (adding enable-global-index = true) or enable it by conf.EnableGlobalIndex = true in a test file.

@L-maple
Copy link
Contributor Author

L-maple commented Jan 29, 2023

@mjonss I find the fix PR: "planner, executor: fix query partition table with global unique index get wrong result (#22478)". The logic is not comfortable for me, as global index hint would be ignored for all situations, and tableFullScan is less efficient.

I still have the following questions:

@L-maple
Copy link
Contributor Author

L-maple commented Feb 7, 2023

In master aa51b46 it does not select the index access, but table scan and gives correct result:

tidb> explain select * from p partition(p0) use index (idx);
+-----------------------+---------+-----------+---------------+----------------------+
| id                    | estRows | task      | access object | operator info        |
+-----------------------+---------+-----------+---------------+----------------------+
| TableReader_5         | 4.00    | root      | partition:p0  | data:TableFullScan_4 |
| └─TableFullScan_4 | 4.00    | cop[tikv] | table:p       | keep order:false     |
+-----------------------+---------+-----------+---------------+----------------------+
2 rows in set (0.00 sec)

tidb> select * from p partition(p0) use index (idx);
+------+------+
| id   | c    |
+------+------+
|    1 |    3 |
+------+------+
1 row in set (0.00 sec)

Note that to test this one needs to enable global index (which is not yet supported). Either by config file (adding enable-global-index = true) or enable it by conf.EnableGlobalIndex = true in a test file.

@mjonss master doesn't solve the problem actually, because it ignore the global index in buildDataSource and the ut is wrong. I solve the problem in my PR: #41159

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tablepartition This issue is related to Table Partition of TiDB. severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants