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

parser: table option should always restore without quotes #38368

Closed
h3n4l opened this issue Oct 10, 2022 · 1 comment · Fixed by #38355
Closed

parser: table option should always restore without quotes #38368

h3n4l opened this issue Oct 10, 2022 · 1 comment · Fixed by #38355
Labels
affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@h3n4l
Copy link
Contributor

h3n4l commented Oct 10, 2022

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

package test
import (
	_ "github.com/pingcap/tidb/types/parser_driver"
	"github.com/stretchr/testify/require"
	"github.com/pingcap/tidb/parser"
	"github.com/pingcap/tidb/parser/ast"
	"github.com/pingcap/tidb/parser/format"
	"github.com/pingcap/tidb/parser/model"
)
func TestTableOption(t *testing.T) {
	tests := []struct {
		new  string
		want string
	}{
		{
			new:  `CREATE TABLE book(id INT) INSERT_METHOD = FIRST;`,
			want: "ALTER TABLE `book` INSERT_METHOD = FIRST;\n",
		},
	}
	a := require.New(t)
	
	for _, test := range tests {
		nodes := parser.New().Parse(test.new, "", "")
		get := deparse(diff, format.DefaultRestoreFlags|format.RestoreStringWithoutCharset)
		a.Equal(test.want, get)
	}
}

func deparse(nodes []ast.Node, flag format.RestoreFlags) (string, error) {
	var buf bytes.Buffer
	for _, node := range nodes {
		if err := node.Restore(format.NewRestoreCtx(flag, &buf)); err != nil {
			return "", err
		}
		if _, err := buf.Write([]byte(";\n")); err != nil {
			return "", err
		}
	}
	return buf.String(), nil
}

We can find that the get is:

ALTER TABLE `book` INSERT_METHOD = 'FIRST';\n

It's caused by ctx.WriteString(n.StrValue) in here. We will encounter the error if we execute it in MySQL client:
image

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

Restore should restore INSERT_METHOD without quotes likes:

CREATE TABLE `tbl`(id INT) INSERT_METHOD = FIRST.

https://dev.mysql.com/doc/refman/8.0/en/create-table.html
CleanShot 2022-10-10 at 17 59 18

3. What did you see instead (Required)

CREATE TABLE `tbl`(id INT) INSERT_METHOD = 'FIRST'.

4. What is your TiDB version? (Required)

Parser at HEAD.

@h3n4l h3n4l added the type/bug The issue is confirmed as a bug. label Oct 10, 2022
@h3n4l
Copy link
Contributor Author

h3n4l commented Oct 10, 2022

I fix it in #38355

@Benjamin2037 Benjamin2037 added the sig/sql-infra SIG: SQL Infra label Oct 10, 2022
@xhebox xhebox added affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 severity/minor labels Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 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.

3 participants