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

support SELECT FROM TABLESAMPLE syntax #1071

Merged
merged 6 commits into from Nov 10, 2020
Merged

Conversation

tangenta
Copy link
Contributor

@tangenta tangenta commented Oct 28, 2020

What problem does this PR solve?

See pingcap/tidb#20567.

What is changed and how it works?

The syntax for "TABLESAMPLE" is as follows:

sample_clause = "TABLESAMPLE" [ "BERNOULLI" | "SYSTEM" | "REGIONS" ] '(' expression [ "PERCENT" | "ROWS" ] ')'
                [ "REPEATABLE" '(' number ')' ]

I put SampleClause as a special part of rule in SelectStmt instead of TableRef, because TableRef is also used in UPDATE/DELETE statements and we don't want to support TABLESAMPLE for them.

However, this also prevent the JOIN clause to be syntax-checked:

select * from tbl1 tablesample (10), tbl2 tablesample (20); -- syntax error
select * from tbl1 a tablesample (10) join tbl2 b tablesample (20) on a.id <> b.id; -- syntax error

Check List

Tests

  • Unit test
  • Integration test

Code changes

N/A

Side effects

N/A

Related changes

N/A

parser.y Outdated Show resolved Hide resolved
parser.y Outdated
@@ -7280,6 +7291,74 @@ SelectStmtFromTable:
$$ = st
}

SelectStmtFromTableSample:
SelectStmtBasic "FROM" TableName PartitionNameListOpt TableAsNameOpt "TABLESAMPLE" TableSampleMethodOpt '(' Expression TableSampleUnitOpt ')' RepeatableOpt WhereClauseOptional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better attach this to this line to support JOIN:

TableFactor:
	TableName PartitionNameListOpt TableAsNameOpt IndexHintListOpt TableSampleOpt # <--

this means the TableSample is part of the ast.TableSource or ast.TableName.

their use in UPDATE and DELETE can be rejected after parsing.

@tangenta tangenta marked this pull request as ready for review October 30, 2020 06:30
Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

ast/dml.go Outdated Show resolved Hide resolved
@kennytm
Copy link
Contributor

kennytm commented Nov 9, 2020

LGTM

@ti-srebot ti-srebot added the status/LGT1 LGT1 label Nov 9, 2020
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

@@ -7805,11 +7888,14 @@ TableRef:
| JoinTable

TableFactor:
TableName PartitionNameListOpt TableAsNameOpt IndexHintListOpt
TableName PartitionNameListOpt TableAsNameOpt IndexHintListOpt TableSampleOpt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should append a warning? since we don't support them all in TiDB actually...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related work is done in TiDB.

Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot removed the status/LGT1 LGT1 label Nov 10, 2020
@ti-srebot ti-srebot added the status/LGT2 LGT2 label Nov 10, 2020
@kennytm kennytm merged commit 8520520 into pingcap:master Nov 10, 2020
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
* support SELECT FROM TABLESAMPLE syntax

* refine naming

* make table sample clause as a part of TableFactor

* address comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants