Skip to content

Support SHOW TABLES LIKE pattern and Where expr #1518

Merged
databend-bot merged 11 commits into
databendlabs:masterfrom
mapleFU:dev-table-likes
Aug 20, 2021
Merged

Support SHOW TABLES LIKE pattern and Where expr #1518
databend-bot merged 11 commits into
databendlabs:masterfrom
mapleFU:dev-table-likes

Conversation

@mapleFU

@mapleFU mapleFU commented Aug 18, 2021

Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the CLA available at: https://datafuse.rs/policies/cla/

Summary

Summary about this PR

This pr supports like 'pattern' and where expr in show tables. I use some interface of sqlparse, and I don't know does I really catch it.

Changelog

  • New Feature

Related Issues

Fixes #1388 #1389

Test Plan

Unit Tests

@databend-bot databend-bot added the pr-feature this PR introduces a new feature to the codebase label Aug 18, 2021
@databend-bot

Copy link
Copy Markdown
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@databend-bot

Copy link
Copy Markdown
Member

Hello @mapleFU, 🎉 Thank you for opening the pull request! 🎉
Your pull request state is not in Draft, please add Reviewers or Re-request review again!
FuseQuery: @BohuTANG @sundy-li @zhang2014
FuseStore: @drmingdrmer @dantengsky
Or visit datafuse roadmap for some clues.

@bohutang
bohutang requested review from leiysky and sundy-li and removed request for sundy-li August 19, 2021 01:08
@sundy-li

Copy link
Copy Markdown
Member

@mapleFU LGTM, better to add a stateless test in 06_0000_show_queries.sql

@codecov-commenter

codecov-commenter commented Aug 19, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1518 (51c7e89) into master (fd1c308) will increase coverage by 0%.
The diff coverage is 65%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #1518    +/-   ##
=======================================
  Coverage      72%     73%            
=======================================
  Files         521     528     +7     
  Lines       31533   32229   +696     
=======================================
+ Hits        22997   23746   +749     
+ Misses       8536    8483    -53     
Impacted Files Coverage Δ
query/src/sql/plan_parser.rs 76% <0%> (-1%) ⬇️
query/src/sql/sql_parser.rs 76% <66%> (-1%) ⬇️
query/src/sql/sql_parser_test.rs 93% <100%> (+<1%) ⬆️
query/src/sql/sql_statement.rs 72% <100%> (+<1%) ⬆️
common/datavalues/src/types/data_type_coercion.rs 70% <0%> (-10%) ⬇️
...pelines/transforms/transform_aggregator_partial.rs 89% <0%> (-2%) ⬇️
query/src/sql/parser/ast/query.rs 46% <0%> (-1%) ⬇️
store/src/meta_service/raftmeta_test.rs 94% <0%> (-1%) ⬇️
common/datavalues/src/series/wrap.rs 61% <0%> (-1%) ⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd1c308...51c7e89. Read the comment docs.

Ok(DfStatement::ShowTables(DfShowTables))
let tok = self.parser.next_token();
match &tok {
Token::EOF => Ok(DfStatement::ShowTables(DfShowTables::All)),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens to the following SQL?

SHOW TABLES  ; 
SHOW TABLES -- comment

@mapleFU mapleFU Aug 19, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will add some tests and check if it's ok tonight.

self.ctx.get_current_database(), i,
)
}
DfShowTables::Where(e) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens to the following SQL?

SHOW TABLES WHERE 1 = 1  OR 1 = 1 

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens to the following SQL?

SHOW TABLES WHERE 1 = 1  OR 1 = 1 

It's ok, we don't care about it, just let it be any complex expr. The optimize / select executor will handle that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

According to MySQL standard,parse_expr can handle all of them: https://dev.mysql.com/doc/refman/5.7/en/expressions.html

Buf I didn't check the code in sqlparse-rs, so I cannot guarantee it works well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May this change the behavior of the query?

After format:
SELECT name FROM system.tables where database = '{}' AND 1 = 1 OR 1 = 1 ORDER BY database, name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

May this change the behavior of the query?

After format:
SELECT name FROM system.tables where database = '{}' AND 1 = 1 OR 1 = 1 ORDER BY database, name

Got It. Sorry for misunderstanding this case! I will adding tests about it in stateless test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

evaluation proceeds left to right, with the exception that assignments evaluate right to left

According to https://dev.mysql.com/doc/refman/8.0/en/operator-precedence.html , it will not return all tables. But I think maybe it's proper to build sql like:

SELECT name FROM system.tables where database = '{}' AND (1 = 1 OR 1 = 1) ORDER BY database, name

@zhang2014 do you think it's ok?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@zhang2014 do you think it's ok?

It looks ok

@mapleFU

mapleFU commented Aug 19, 2021

Copy link
Copy Markdown
Contributor Author

@zhang2014 @sundy-li Sorry for fixing the problem so late!

I add some tests in stateless, and wrap where expr with ().

There are some cases:

  1. commit d9eaee0 remove the handling about ;, because it was already processed.
  2. When I build sql like from system.tables where ... name = t or 1 = 1 without (), some system table was printed. I have tried it on MySQL but mysql will not print these messages.

@sundy-li sundy-li left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks @mapleFU

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot
databend-bot merged commit 5eb2ba8 into databendlabs:master Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature this PR introduces a new feature to the codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support SHOW TABLES LIKE pattern and Where expr

5 participants