Skip to content

Add MySql, BigQuery to all dialects #697

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

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Conversation

omer-shtivi
Copy link
Contributor

@omer-shtivi omer-shtivi commented Oct 31, 2022

close #690

@omer-shtivi
Copy link
Contributor Author

@AugustoFKL , @alamb
MySql doesn't seem to support

SELECT "alias"."bar baz", "myfun"(), "simple id" AS "column alias" FROM "a table" AS "alias"

in test parse_delimited_identifiers
do you prefer to move this test to another dialect? or somehow exclude the MySql from all dialects?

@omer-shtivi omer-shtivi reopened this Oct 31, 2022
@AugustoFKL
Copy link
Contributor

@omer-shtivi I'd say to move the test, as MySQL doesn't accept 'x' as an identifier

Thanks for adding MySQL and BigQuery to all tests.

@omer-shtivi
Copy link
Contributor Author

If we want to support parse_similar_to and parse_like in mysql, we need to support getting the an-escaped char.
For example:

SELECT * FROM customers WHERE name {}LIKE '%a' ESCAPE '\'

at mysql needs to be:

SELECT * FROM customers WHERE name {}LIKE '%a' ESCAPE '\\'

the \ is escaped using double backslashes

@omer-shtivi omer-shtivi marked this pull request as ready for review November 3, 2022 18:31
@AugustoFKL
Copy link
Contributor

@omer-shtivi I think this can be directed to another issue or PR.

Copy link
Contributor

@AugustoFKL AugustoFKL left a comment

Choose a reason for hiding this comment

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

I think that LGTM.

@alamb ready for your look :)

This is in essence a little refactor, right @omer-shtivi ?

Copy link
Contributor

@AugustoFKL AugustoFKL left a comment

Choose a reason for hiding this comment

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

(sorry the previous comment, missclick)
I think that LGTM.

@alamb ready for your look :)

This is in essence a little refactor, right @omer-shtivi ?

@omer-shtivi
Copy link
Contributor Author

Hi @AugustoFKL ,
Yes, it mainly testing refactor to support all dialects

@alamb
Copy link
Contributor

alamb commented Nov 7, 2022

Thank you @omer-shtivi

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3388319891

  • 575 of 635 (90.55%) changed or added relevant lines in 9 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.2%) to 86.167%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tests/sqlparser_bigquery.rs 50 56 89.29%
tests/sqlparser_sqlite.rs 49 55 89.09%
tests/sqlparser_clickhouse.rs 79 87 90.8%
tests/sqlparser_hive.rs 79 87 90.8%
tests/sqlparser_mssql.rs 79 87 90.8%
tests/sqlparser_postgres.rs 79 87 90.8%
tests/sqlparser_redshift.rs 79 87 90.8%
tests/sqlparser_snowflake.rs 79 87 90.8%
Files with Coverage Reduction New Missed Lines %
src/parser.rs 1 83.77%
src/lib.rs 2 10.0%
Totals Coverage Status
Change from base Build 3385991687: 0.2%
Covered Lines: 11530
Relevant Lines: 13381

💛 - Coveralls

@alamb alamb merged commit 0428ac7 into apache:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix test-util fn all_dialects() to test all dialects actually
4 participants