GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Closes #1604, added simple OVERLAPS support by d2a-raudenaerde · Pull Request #1611 · JSQLParser/JSqlParser

Conversation

@d2a-raudenaerde

@d2a-raudenaerde d2a-raudenaerde commented Aug 4, 2022

Copy link
Copy Markdown
Contributor
  • Using the ExpressionList approach as discussed in the other PR
  • No renames of existing classes
  • Moved to SQLCondition

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@manticore-projects

manticore-projects commented Aug 4, 2022

Copy link
Copy Markdown
Contributor

Rob,

there was an unrelated PR merged last night, which breaks the tests.
I have provided a fix already, but we will need to wait for it to get merged first.

Until this happens, all our PRs will fail to built. So please a bit patience right now. Thanks, its not in my hands.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@d2a-raudenaerde

d2a-raudenaerde commented Aug 4, 2022

Copy link
Copy Markdown
Contributor Author

I noticed. I spent some time trying to figure out why the AlterTest was failing, until I decided to check if the main branch was the issue, and it was.

(You're still welcome to provide code-review-feedback already of course)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@manticore-projects

manticore-projects commented Aug 4, 2022

Copy link
Copy Markdown
Contributor

I definitely will, just wait please until Master builds again.
It wasn't me but it affects my work as well.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@manticore-projects

manticore-projects commented Aug 4, 2022

Copy link
Copy Markdown
Contributor

It's a thing of beauty now and well executed!

One recommendation: You may want to add <OVERLAPS> to the RelObjectWithoutValue Production in order to allow it as Column, Table, Alias, Function. (I still hope that my Keywords PR will be allowed to do that automatically eventually.)

One question: Are those ExpressionList() really of a variable length? Or are they limited to strictly 2? Should we want to enforce it?

Well done and thank you for your contribution!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@d2a-raudenaerde

d2a-raudenaerde commented Aug 4, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Thanks :)

You meant: RelObjectNameWithoutValue right? I'm not really sure if that is correct/something desired? At least PostgreSQL does not allow it to be used as Table without quoting it.

So CREATE TABLE OVERLAPS(A TEXT); gives a syntax error; CREATE TABLE "OVERLAPS"(A TEXT); is allowed.

If there is consensus that it should be allowed, I'm happy to add it! (or maybe, after the keyword PR / discussion is settled?)

About the ExpressionList: I tried to stick to the sql2003 standard. I'm not too confident about the different DBMS handling of OVERLAPS. The standard allows a list of size 2+, or, when prefixed with ROW, a list of size 1+. The latter I did not implement as I tried to follow the mantra 'we will implement it when needed' :)

WDYT?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@manticore-projects

manticore-projects commented Aug 4, 2022

Copy link
Copy Markdown
Contributor

You meant: RelObjectNameWithoutValue right?
Yes.

I'm not really sure if that is correct/something desired? At least PostgreSQL does not allow it to be used as Table without quoting it.

So CREATE TABLE OVERLAPS(A TEXT); gives a syntax error; CREATE TABLE "OVERLAPS"(A TEXT); is allowed.

As long as it is not explicitly defined by the SQL Standard as a Reserved Keyword we should try to allow it as a keyword.

If there is consensus that it should be allowed, I'm happy to add it! (or maybe, after the keyword PR / discussion is settled?)

I have no particular interest in this one Keyword. But I wanted to point out the mechanism -- for your next great contribution.

About the ExpressionList: I tried to stick to the sql2003 standard. I'm not too confident about the different DBMS handling of OVERLAPS. The standard allows a list of size 2+,

Perfect then, this was all I wanted to know. (I have never used OVERLAPS and I am still very stuck with Oracle and SQL 2002, sorry.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@d2a-raudenaerde

d2a-raudenaerde commented Aug 4, 2022

Copy link
Copy Markdown
Contributor Author

I have no particular interest in this one Keyword. But I wanted to point out the mechanism -- for your next great contribution.

I will leave this for now, if there arises an issue/request for it, we can easily add it.

Perfect then, this was all I wanted to know

Thanks for the question; I'll leave a comment in the grammar pointing this out, that's useful for other ppl. that might wonder why this has been done this way.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@d2a-raudenaerde
Closes JSQLParser#1604, added simple OVERLAPS support
68a4d92
@d2a-raudenaerde
d2a-raudenaerde force-pushed the 1604-take-2 branch from 43296ce to 68a4d92 Compare August 4, 2022 15:10
@d2a-raudenaerde
Merge branch 'JSQLParser:master' into 1604-take-2
5ae3a28
@d2a-raudenaerde

d2a-raudenaerde commented Aug 16, 2022

Copy link
Copy Markdown
Contributor Author

@manticore-projects can this be pulled now? :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@manticore-projects

manticore-projects commented Aug 16, 2022 via email

Copy link
Copy Markdown
Contributor
Yes, it certainly can. However only @wumpz can merge.On 16 Aug 2022 08:50, Rob Audenaerde ***@***.***> wrote: @manticore-projects can this be pulled now? :) —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@wumpz
wumpz merged commit 236a50b into JSQLParser:master Aug 16, 2022
@wumpz

wumpz commented Aug 16, 2022

Copy link
Copy Markdown
Member

Did a full beach merge ... :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@d2a-raudenaerde

d2a-raudenaerde commented Aug 16, 2022

Copy link
Copy Markdown
Contributor Author

Ha, enjoy your beach-time!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@manticore-projects

manticore-projects commented Aug 18, 2022

Copy link
Copy Markdown
Contributor

You are my hero, Sir!

Aug 18, 2022 8:10:28 PM net.sf.jsqlparser.statement.select.SpecialOracleTest recordSuccessOnSourceFile
INFO: NEW SUCCESS: interval05.sql
Aug 18, 2022 8:10:29 PM net.sf.jsqlparser.statement.select.SpecialOracleTest testAllSqlsParseDeparse
INFO: tested 276 files. got 192 correct parse results, expected 191

You increased the Oracle Coverage/Compliance by 1 test -- and nobody has noticed.
Please go to src/test/java/net/sf/jsqlparser/statement/select/SpecialOracleTest.java and register your succeeding test as I do not want to steal the merit.

Thank you again for the contribution and cheers!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@d2a-raudenaerde

d2a-raudenaerde commented Aug 19, 2022

Copy link
Copy Markdown
Contributor Author

I really don't mind anybody 'stealing' the merit, so go ahead :)

(I just care about having a great tool to help me in my SQL conquests, and am happy to be able to contribute where/when possible)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@d2a-raudenaerde @manticore-projects @wumpz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.