-
Notifications
You must be signed in to change notification settings - Fork 38
#58 -AS clause always injected in generated SQL #59
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for the PRs. I will take a look at all the PR activity and merge changes in at the end of this week. |
|
Thanks Tyler.
…On Tue, May 27, 2025 at 1:21 AM Tyler Brinks ***@***.***> wrote:
*TylerBrinks* left a comment (TylerBrinks/SqlParser-cs#59)
<#59 (comment)>
Thanks for the PRs. I will take a look at all the PR activity and merge
changes in at the end of this week.
—
Reply to this email directly, view it on GitHub
<#59 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMWUXYQFER3YT2MTOULMJGD3AOOW5AVCNFSM6AAAAAB52O6W7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMJQG42TIMZTGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Hi Tyler,
Sorry to bother you.. any news for me?
Thanks in advance
…On Tue, May 27, 2025 at 9:08 AM Luigi Esposito ***@***.***> wrote:
Thanks Tyler.
On Tue, May 27, 2025 at 1:21 AM Tyler Brinks ***@***.***>
wrote:
> *TylerBrinks* left a comment (TylerBrinks/SqlParser-cs#59)
> <#59 (comment)>
>
> Thanks for the PRs. I will take a look at all the PR activity and merge
> changes in at the end of this week.
>
> —
> Reply to this email directly, view it on GitHub
> <#59 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AMWUXYQFER3YT2MTOULMJGD3AOOW5AVCNFSM6AAAAAB52O6W7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMJQG42TIMZTGA>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
Looking through the Rust project, the current main branch has a widely accepted suite of passing tests that contradict most of the AS changes in this PR. I don't disagree that some dialects may need to handle the keyword differently, but these changes deviate from the accepted dialect tests for both projects. I'd suggest opening a dialog with the Rust project team (which I contribute to) to discuss whether there is indeed an issue, and if so with which dialect. |
|
Hi Tyller,
The issue anyway is that AS keyword raise exception on Oracle DBMS.
The solution I proposed is simply to check is that AS keyword was present
on the original SQL statement.
If you have or think to other solution I am open to it
Regards
Luigi
…On Thu, Jun 5, 2025 at 4:17 AM Tyler Brinks ***@***.***> wrote:
*TylerBrinks* left a comment (TylerBrinks/SqlParser-cs#59)
<#59 (comment)>
Looking through the Rust project, the current main branch has a widely
accepted suite of passing tests that contradict most of the AS changes in
this PR. I don't disagree that some dialects may need to handle the keyword
differently, but these changes deviate from the accepted dialect tests for
both projects. I'd suggest opening a dialog with the Rust project team
(which I contribute to) to discuss whether there is indeed an issue, and if
so with which dialect.
For example,
https://github.com/apache/datafusion-sqlparser-rs/blob/5327f0ce132e12de71db7d03711397c5ac6c0031/tests/sqlparser_snowflake.rs#L1035
—
Reply to this email directly, view it on GitHub
<#59 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMWUXYSH2I7PDSW5KACJ5TD3B6SEDAVCNFSM6AAAAAB52O6W7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNBSGUZTANJSGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Hi Tyler,
I am using Generic dialect for Oracle which is not the right thing but
unfortunately I didn't find a way to create a dedicated
dialect class and override the AS added by default.
Probably the Rust team will have same issue if they try to check SQL
statement generated on Oracle.
Regards
Luigi
…On Thu, Jun 5, 2025 at 1:53 PM Luigi Esposito ***@***.***> wrote:
Hi Tyller,
The issue anyway is that AS keyword raise exception on Oracle DBMS.
The solution I proposed is simply to check is that AS keyword was present
on the original SQL statement.
If you have or think to other solution I am open to it
Regards
Luigi
On Thu, Jun 5, 2025 at 4:17 AM Tyler Brinks ***@***.***>
wrote:
> *TylerBrinks* left a comment (TylerBrinks/SqlParser-cs#59)
> <#59 (comment)>
>
> Looking through the Rust project, the current main branch has a widely
> accepted suite of passing tests that contradict most of the AS changes in
> this PR. I don't disagree that some dialects may need to handle the keyword
> differently, but these changes deviate from the accepted dialect tests for
> both projects. I'd suggest opening a dialog with the Rust project team
> (which I contribute to) to discuss whether there is indeed an issue, and if
> so with which dialect.
>
> For example,
>
> https://github.com/apache/datafusion-sqlparser-rs/blob/5327f0ce132e12de71db7d03711397c5ac6c0031/tests/sqlparser_snowflake.rs#L1035
>
> —
> Reply to this email directly, view it on GitHub
> <#59 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AMWUXYSH2I7PDSW5KACJ5TD3B6SEDAVCNFSM6AAAAAB52O6W7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNBSGUZTANJSGU>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
This solves issue #58