expression: add builtin function tidb_parse_tso #8385

yu34po · 2018-11-21T08:22:58Z

What problem does this PR solve?

#8384

What is changed and how it works?

add time builtin function tso

Check List

Unit test
Integration test

Code changes

Has exported function/method change
Has exported variable/fields change

Side effects

Related changes

This change is

sre-bot · 2018-11-21T08:23:03Z

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

jackysp · 2018-11-21T08:27:36Z

/ok-to-test

jackysp · 2018-11-21T08:30:13Z

Thanks for your contribution, @yu34po !
Please fix CI, seems the failure is about the timezone.

jackysp · 2018-11-21T10:02:22Z

/run-all-tests

expression/builtin_time.go

dbjoa · 2018-11-22T02:11:03Z

expression/builtin_time.go

+// evalTime evals a builtinTsoSig.
+func (b *builtinTsoSig) evalTime(row chunk.Row) (types.Time, bool, error) {
+	arg, isNull, err := b.args[0].EvalInt(b.ctx, row)
+	if isNull || err != nil || arg <= 0 {


Why should we consider the case of isNull == true as an error ?

function must accept a parameter

In the statement: "update t1,t2 set t1.id = t2.id" TiDB should check update privilege for t1 and select privilege for t2, Fix a bug that it checks update privilege for both t1 and t2

…ingcap#8297)

dbjoa · 2018-11-22T07:14:33Z

@yu34po The PR includes vender files, which should not be. Would you check whether or not you set the environment variable, GO111MODULE=on?

dbjoa · 2018-11-22T08:05:54Z

@yu34po Would you also add the test for SQL statements having tidb_parse_tso()? expression/integration_test.go can be a candidate place for the test.

XuHuaiyu · 2018-11-22T08:34:22Z

expression/builtin.go

@@ -413,6 +413,7 @@ var funcs = map[string]functionClass{
 	ast.Year:             &yearFunctionClass{baseFunctionClass{ast.Year, 1, 1}},
 	ast.YearWeek:         &yearWeekFunctionClass{baseFunctionClass{ast.YearWeek, 1, 2}},
 	ast.LastDay:          &lastDayFunctionClass{baseFunctionClass{ast.LastDay, 1, 1}},
+	ast.Tso:              &tsoFunctionClass{baseFunctionClass{ast.Tso, 1, 1}},


We'd better change ast.Tso to ast.TidbParseTso

ast.Tso already merged in parser master

We can push another PR to change the name

expression/builtin_time.go

zz-jason

~~LGTM~~

zz-jason · 2018-11-22T10:18:08Z

@XuHuaiyu @dbjoa PTAL

XuHuaiyu

I think we'd better:

change ast.Tso to ast.TiDBParseTso
change tsoXXX to tidbParseTsoXXX

dbjoa · 2018-11-23T04:44:41Z

LGTM

XuHuaiyu

Please address the comment.
Rest LGTM.
Thanks for your contribution.

expression/builtin_time.go

zz-jason · 2018-11-24T09:11:41Z

go.mod

@@ -49,3 +50,5 @@ require (
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0
 	sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4
 )
+
+replace github.com/pingcap/parser => github.com/yu34po/parser v0.0.0-20181123030610-1d6c3c585f2d


please update go.mod to use the latest pingcap/parser.

zz-jason

please update go.mod

zz-jason · 2018-11-26T11:17:47Z

@yu34po pingcap/parser#50 has been merged, you can update go.mod to the latest pingcap/parser now.

zz-jason · 2018-11-27T03:14:44Z

go.mod

@@ -41,6 +41,7 @@ require (
 	github.com/shirou/gopsutil v2.18.10+incompatible
 	github.com/sirupsen/logrus v1.2.0
 	github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72
+	github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2


why add this?

I don't know how this happened.
same as master expect parser version

It disappeared after merging the master branch..

zz-jason

LGTM

zz-jason · 2018-11-28T14:01:24Z

/run-all-tests

add builtin function from_tso

bb6af76

Merge branch 'master' into tsofunc

4ac2edd

jackysp added contribution This PR is from a community contributor. component/expression labels Nov 21, 2018

add timezone convert

4746d72

dbjoa reviewed Nov 22, 2018

View reviewed changes

expression/builtin_time.go Outdated Show resolved Hide resolved

dbjoa reviewed Nov 22, 2018

View reviewed changes

gregwebs and others added 6 commits November 22, 2018 11:26

Update dockerfile (pingcap#8363)

43d4cd8

planner/core,session: fix privilege check for update (pingcap#8376)

e924347

In the statement: "update t1,t2 set t1.id = t2.id" TiDB should check update privilege for t1 and select privilege for t2, Fix a bug that it checks update privilege for both t1 and t2

store/kv: fix panic for EOF-stream & add recover to copIterateWorker (p…

8361903

…ingcap#8297)

add builtin function from_tso

0c675c7

delete unused line

9bcd7fc

Merge branch 'master' into tsofunc

ca97bb1

del vendor

953f592

add integration test

d982a4c

XuHuaiyu reviewed Nov 22, 2018

View reviewed changes

XuHuaiyu changed the title ~~add builtin function from_tso~~ expression: add builtin function tidb_parse_tso Nov 22, 2018

XuHuaiyu reviewed Nov 22, 2018

View reviewed changes

expression/builtin_time.go Outdated Show resolved Hide resolved

add comment

187677c

zz-jason reviewed Nov 22, 2018

View reviewed changes

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

XuHuaiyu requested changes Nov 22, 2018

View reviewed changes

change tso name to TiDBParseTso

3f268da

XuHuaiyu reviewed Nov 23, 2018

View reviewed changes

expression/builtin_time.go Outdated Show resolved Hide resolved

chnage sig name

71d0527

zz-jason reviewed Nov 24, 2018

View reviewed changes

XuHuaiyu previously approved these changes Nov 26, 2018

View reviewed changes

Merge branch 'master' into tsofunc

f2d7c13

zz-jason requested changes Nov 26, 2018

View reviewed changes

tiancaiamao mentioned this pull request Nov 26, 2018

change Tso function name to TiDBParseTso pingcap/parser#50

Merged

change go.mod parser to master branch

6085ab3

yu34po dismissed XuHuaiyu’s stale review via 6085ab3 November 27, 2018 02:54

yu34po added 2 commits November 27, 2018 11:04

clean go.mod go.sum

4b56b52

rollback go.mod

48564d6

zz-jason reviewed Nov 27, 2018

View reviewed changes

zz-jason added 2 commits November 27, 2018 16:26

Merge branch 'master' into tsofunc

b71e130

Merge branch 'master' into tsofunc

42e7f88

zz-jason approved these changes Nov 28, 2018

View reviewed changes

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

zz-jason merged commit 2f6639d into pingcap:master Nov 28, 2018

dveeden mentioned this pull request May 26, 2021

Add documentation for TiDB specific SQL functions pingcap/docs#5699

Merged

6 tasks

ti-chi-bot mentioned this pull request Jun 3, 2021

Add documentation for TiDB specific SQL functions (#5699) pingcap/docs#5751

Merged

6 tasks

Liuxiaozhen12 mentioned this pull request Jun 24, 2021

Add documentation for TiDB specific SQL functions pingcap/docs-cn#6520

Merged

11 tasks

This was referenced Jul 5, 2021

Add documentation for TiDB specific SQL functions (#6520) pingcap/docs-cn#6611

Merged

Add documentation for TiDB specific SQL functions (#6520) pingcap/docs-cn#6612

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: add builtin function tidb_parse_tso #8385

expression: add builtin function tidb_parse_tso #8385

yu34po commented Nov 21, 2018 •

edited by c4pt0r

Loading

sre-bot commented Nov 21, 2018

jackysp commented Nov 21, 2018

jackysp commented Nov 21, 2018

jackysp commented Nov 21, 2018

dbjoa Nov 22, 2018 •

edited

Loading

yu34po Nov 22, 2018

dbjoa Nov 23, 2018

dbjoa commented Nov 22, 2018

dbjoa commented Nov 22, 2018

XuHuaiyu Nov 22, 2018

yu34po Nov 22, 2018

XuHuaiyu Nov 22, 2018

zz-jason left a comment •

edited

Loading

zz-jason commented Nov 22, 2018

XuHuaiyu left a comment

dbjoa commented Nov 23, 2018

XuHuaiyu left a comment

zz-jason Nov 24, 2018

zz-jason left a comment

zz-jason commented Nov 26, 2018

zz-jason Nov 27, 2018

yu34po Nov 27, 2018

zz-jason Nov 28, 2018 •

edited

Loading

zz-jason left a comment

zz-jason commented Nov 28, 2018

expression: add builtin function tidb_parse_tso #8385

expression: add builtin function tidb_parse_tso #8385

Conversation

yu34po commented Nov 21, 2018 • edited by c4pt0r Loading

What problem does this PR solve?

What is changed and how it works?

Check List

sre-bot commented Nov 21, 2018

jackysp commented Nov 21, 2018

jackysp commented Nov 21, 2018

jackysp commented Nov 21, 2018

dbjoa Nov 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbjoa commented Nov 22, 2018

dbjoa commented Nov 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason left a comment • edited Loading

Choose a reason for hiding this comment

zz-jason commented Nov 22, 2018

XuHuaiyu left a comment

Choose a reason for hiding this comment

dbjoa commented Nov 23, 2018

XuHuaiyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason left a comment

Choose a reason for hiding this comment

zz-jason commented Nov 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

zz-jason left a comment

Choose a reason for hiding this comment

zz-jason commented Nov 28, 2018

yu34po commented Nov 21, 2018 •

edited by c4pt0r

Loading

dbjoa Nov 22, 2018 •

edited

Loading

zz-jason left a comment •

edited

Loading

zz-jason Nov 28, 2018 •

edited

Loading