ranger: refine explain format, again #7041

zz-jason · 2018-07-12T12:07:43Z

What have you changed? (mandatory)

Refine the string format of ranges and constant expressions:
- use NULL to represent NULL values instead of <nil>
- wrap string with "
Show whether we use the pseudo statistics for a table or index in the explain result.
Refine the count and range information for inner table of IndexJoin

Before this PR:

TiDB(localhost:4000) > desc select * from t where a = "<nil>";
+-------------------+-------+------+---------------------------------------------------------+
| id                | count | task | operator info                                           |
+-------------------+-------+------+---------------------------------------------------------+
| IndexReader_9     | 10.00 | root | index:IndexScan_8                                       |
| └─IndexScan_8     | 10.00 | cop  | table:t, index:a, range:[<nil>,<nil>], keep order:false |
+-------------------+-------+------+---------------------------------------------------------+
2 rows in set (0.01 sec)

TiDB(localhost:4000) > desc select * from t where a = "NULL";
+-------------------+-------+------+-------------------------------------------------------+
| id                | count | task | operator info                                         |
+-------------------+-------+------+-------------------------------------------------------+
| IndexReader_9     | 10.00 | root | index:IndexScan_8                                     |
| └─IndexScan_8     | 10.00 | cop  | table:t, index:a, range:[NULL,NULL], keep order:false |
+-------------------+-------+------+-------------------------------------------------------+
2 rows in set (0.00 sec)

TiDB(localhost:4000) > desc select * from t where a is null;
+-------------------+-------+------+---------------------------------------------------------+
| id                | count | task | operator info                                           |
+-------------------+-------+------+---------------------------------------------------------+
| IndexReader_9     | 10.00 | root | index:IndexScan_8                                       |
| └─IndexScan_8     | 10.00 | cop  | table:t, index:a, range:[<nil>,<nil>], keep order:false |
+-------------------+-------+------+---------------------------------------------------------+
2 rows in set (0.00 sec)

TiDB(localhost:4000) > desc select /*+ TIDB_INLJ(t1) */ t2.a from t1 join t2 on t1.a=t2.a;
+--------------------------+----------+------+---------------------------------------------------------------------------+
| id                       | count    | task | operator info                                                             |
+--------------------------+----------+------+---------------------------------------------------------------------------+
| Projection_5             | 12500.00 | root | test.t2.a                                                                 |
| └─IndexJoin_9            | 12500.00 | root | inner join, inner:TableReader_8, outer key:test.t1.a, inner key:test.t2.a |
|   ├─TableReader_11       | 10000.00 | root | data:TableScan_10                                                         |
|   │ └─TableScan_10       | 10000.00 | cop  | table:t1, range:[-inf,+inf], keep order:false                             |
|   └─TableReader_8        | 10000.00 | root | data:TableScan_7                                                          |
|     └─TableScan_7        | 10000.00 | cop  | table:t2, range:[-inf,+inf], keep order:false                             |
+--------------------------+----------+------+---------------------------------------------------------------------------+
6 rows in set (0.00 sec)

After this PR:

TiDB(localhost:4000) > desc select * from t where a = "<nil>";
+-------------------+-------+------+---------------------------------------------------------------------------+
| id                | count | task | operator info                                                             |
+-------------------+-------+------+---------------------------------------------------------------------------+
| IndexReader_9     | 10.00 | root | index:IndexScan_8                                                         |
| └─IndexScan_8     | 10.00 | cop  | table:t, index:a, range:["<nil>","<nil>"], keep order:false, stats:pseudo |
+-------------------+-------+------+---------------------------------------------------------------------------+
2 rows in set (0.00 sec)

TiDB(localhost:4000) > desc select * from t where a = "NULL";
+-------------------+-------+------+-------------------------------------------------------------------------+
| id                | count | task | operator info                                                           |
+-------------------+-------+------+-------------------------------------------------------------------------+
| IndexReader_9     | 10.00 | root | index:IndexScan_8                                                       |
| └─IndexScan_8     | 10.00 | cop  | table:t, index:a, range:["NULL","NULL"], keep order:false, stats:pseudo |
+-------------------+-------+------+-------------------------------------------------------------------------+
2 rows in set (0.00 sec)

TiDB(localhost:4000) > desc select * from t where a is null;
+-------------------+-------+------+---------------------------------------------------------------------+
| id                | count | task | operator info                                                       |
+-------------------+-------+------+---------------------------------------------------------------------+
| IndexReader_9     | 10.00 | root | index:IndexScan_8                                                   |
| └─IndexScan_8     | 10.00 | cop  | table:t, index:a, range:[NULL,NULL], keep order:false, stats:pseudo |
+-------------------+-------+------+---------------------------------------------------------------------+
2 rows in set (0.00 sec)

TiDB(localhost:4000) > desc select /*+ TIDB_INLJ(t1) */ t2.a from t1 join t2 on t1.a=t2.a;
+--------------------------+----------+------+---------------------------------------------------------------------------+
| id                       | count    | task | operator info                                                             |
+--------------------------+----------+------+---------------------------------------------------------------------------+
| Projection_5             | 12500.00 | root | test.t2.a                                                                 |
| └─IndexJoin_9            | 12500.00 | root | inner join, inner:TableReader_8, outer key:test.t1.a, inner key:test.t2.a |
|   ├─TableReader_11       | 10000.00 | root | data:TableScan_10                                                         |
|   │ └─TableScan_10       | 10000.00 | cop  | table:t1, range:[-inf,+inf], keep order:false, stats:pseudo               |
|   └─TableReader_8        | 10.00    | root | data:TableScan_7                                                          |
|     └─TableScan_7        | 10.00    | cop  | table:t2, range: decided by [test.t1.a], keep order:false, stats:pseudo   |
+--------------------------+----------+------+---------------------------------------------------------------------------+
6 rows in set (0.01 sec)

What is the type of the changes? (mandatory)

Improvement (non-breaking change which is an improvement to an existing feature)

How has this PR been tested? (mandatory)

unit test
explain test

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Does this PR affect tidb-ansible update? (mandatory)

Does this PR need to be added to the release notes? (mandatory)

Refer to a related PR or issue link (optional)

fix #6935
fix #7038

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

zz-jason · 2018-07-12T12:08:00Z

@winoros PTAL

shenli · 2018-07-12T14:19:23Z

Please fix the CI.

alivxxx · 2018-07-13T05:58:40Z

util/ranger/types.go

@@ -130,6 +130,9 @@ func formatDatum(d types.Datum, isLeftSide bool) string {
 		if d.GetUint64() == math.MaxUint64 && !isLeftSide {
 			return "+inf"
 		}
+	case types.KindString, types.KindBytes, types.KindMysqlEnum, types.KindMysqlSet,
+		types.KindMysqlJSON, types.KindBinaryLiteral, types.KindMysqlBit:
+		return fmt.Sprintf("\"%v\"", d.GetValue())
 	}
 	return fmt.Sprintf("%v", d.GetValue())


Why not just wrap it here?

it's leaved for other types, for example decimal/double/datetime/timestamp, I think they'd better not be warped with "

zz-jason · 2018-07-16T13:42:53Z

@winoros @lamxTyler PTAL

zz-jason · 2018-07-16T13:50:07Z

/run-all-tests

winoros · 2018-07-16T15:44:10Z

util/ranger/ranger_test.go

 		},
 		{
 			indexPos:    0,
 			exprStr:     `a LIKE "\\\\a%"`,
 			accessConds: `[like(test.t.a, \\a%, 92)]`,
 			filterConds: "[]",
-			resultStr:   `[[\a <nil>,\b <nil>)]`,
+			resultStr:   "[[\"\\a\" NULL,\"\\b\" NULL)]",


Why change the quotation?

this is due to the modification of formatDatum in util/ranger/types.go

i mean use `, we don't need escape characters

Oh, the result was directly copied from the test output😂

The usage of ` and " is not uniformed now, there are many tests use " actually.

shenli · 2018-07-16T15:50:28Z

@lamxTyler PTAL

alivxxx

LGTM

…lain-range

zz-jason · 2018-07-17T07:38:33Z

@winoros PTAL

coocood · 2018-07-17T08:02:12Z

plan/exhaust_physical_plans.go

+	if ok && !ds.statisticTable.Pseudo {
+		rowCount = idxHist.AvgCountPerValue(ds.statisticTable.Count)
+	} else {
+		rowCount = ds.statisticTable.PseudoAvgCountPerCount()


The method name should be PseudoAvgCountPerValue?

yes, already changed.

…into dev/explain-range

zz-jason · 2018-07-17T08:36:31Z

@coocood PTAL

coocood · 2018-07-17T08:44:41Z

LGTM

ranger: refine the string format of ranges

ca9a0e8

zz-jason added type/enhancement The issue or PR belongs to an enhancement. sig/planner SIG: Planner labels Jul 12, 2018

alivxxx reviewed Jul 13, 2018

View reviewed changes

zz-jason added 2 commits July 16, 2018 14:36

Merge branch 'master' into dev/explain-range

52ddb99

fix ci

dd559af

zz-jason changed the title ~~ranger: refine the string format of ranges~~ ranger: refine explain format, again Jul 16, 2018

Merge branch 'master' into dev/explain-range

2d5d85a

winoros reviewed Jul 16, 2018

View reviewed changes

zz-jason added the status/all tests passed label Jul 16, 2018

alivxxx reviewed Jul 17, 2018

View reviewed changes

zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 17, 2018

zz-jason added 2 commits July 17, 2018 15:38

Merge branch 'master' of https://github.com/pingcap/tidb into dev/exp…

ad3d475

…lain-range

Merge branch 'master' into dev/explain-range

6dfd7ad

coocood reviewed Jul 17, 2018

View reviewed changes

zz-jason added 2 commits July 17, 2018 16:35

address comment

d30cddb

Merge branch 'dev/explain-range' of https://github.com/zz-jason/tidb …

d832836

…into dev/explain-range

zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 17, 2018

coocood approved these changes Jul 17, 2018

View reviewed changes

zz-jason merged commit 1bf3f3d into pingcap:master Jul 17, 2018

zz-jason deleted the dev/explain-range branch July 17, 2018 08:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ranger: refine explain format, again #7041

ranger: refine explain format, again #7041

zz-jason commented Jul 12, 2018 •

edited

Loading

zz-jason commented Jul 12, 2018

shenli commented Jul 12, 2018

alivxxx Jul 13, 2018

zz-jason Jul 13, 2018

zz-jason commented Jul 16, 2018

zz-jason commented Jul 16, 2018

winoros Jul 16, 2018

zz-jason Jul 16, 2018

winoros Jul 16, 2018

zz-jason Jul 16, 2018

zz-jason Jul 17, 2018

shenli commented Jul 16, 2018

alivxxx left a comment

zz-jason commented Jul 17, 2018

coocood Jul 17, 2018

zz-jason Jul 17, 2018

zz-jason commented Jul 17, 2018

coocood commented Jul 17, 2018

ranger: refine explain format, again #7041

ranger: refine explain format, again #7041

Conversation

zz-jason commented Jul 12, 2018 • edited Loading

What have you changed? (mandatory)

What is the type of the changes? (mandatory)

How has this PR been tested? (mandatory)

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Does this PR affect tidb-ansible update? (mandatory)

Does this PR need to be added to the release notes? (mandatory)

Refer to a related PR or issue link (optional)

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

zz-jason commented Jul 12, 2018

shenli commented Jul 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason commented Jul 16, 2018

zz-jason commented Jul 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shenli commented Jul 16, 2018

alivxxx left a comment

Choose a reason for hiding this comment

zz-jason commented Jul 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason commented Jul 17, 2018

coocood commented Jul 17, 2018

zz-jason commented Jul 12, 2018 •

edited

Loading