Skip to content

Conversation

opeongo
Copy link

@opeongo opeongo commented Jul 13, 2016

This pull request implements the Postgres SQL ARRAY literal, and the ANY and ALL array operators.

opeongo added 3 commits July 12, 2016 09:59
…operator. This in not complete yet because the implementation has a syntax conflict, so it has disabled identifiers quoted with [] characters.
@coveralls
Copy link

coveralls commented Jul 13, 2016

Coverage Status

Coverage decreased (-0.9%) to 82.925% when pulling 084aa35 on opeongo:arrayLiteral into 04d2b65 on JSQLParser:master.

@wumpz
Copy link
Member

wumpz commented Jul 15, 2016

I will look into it. At the moment I am nearly full time offline. You know holidays. :) Cheers Tobias

@opeongo
Copy link
Author

opeongo commented Sep 27, 2016

Hi Tobias,

I'm just wondering if you have any interest in this feature, or is it a
dead end?

On Fri, Jul 15, 2016 at 9:40 AM, Tobias notifications@github.com wrote:

I will look into it. At the moment I am nearly full time offline. You know
holidays. :) Cheers Tobias


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#306 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMgvTW3LO_u9hmHv9dVY538RFg_f3Kkoks5qV43rgaJpZM4JL4ed
.

@wumpz
Copy link
Member

wumpz commented Oct 3, 2016

Sorry. I overlooked your pr after beeing back from holidays.

What I do not like about your change is this kind of token "array whitespace* [". I have to admit, that I do not have a proper solution at hand. Thats the reason your pr is not merged. At the moment I do not think, that a "correct" (in my eyes ;)) solution and the "bracket" quotation of identifiers can coexist. Maybe one could construct a solution using syntactic or token lookahead .. Tobias

@opeongo
Copy link
Author

opeongo commented Oct 5, 2016

I agree that it would be aesthetically more pleasing if token did not use
the '[' character, but I was not able to figure out how to do it
otherwise. I have a limited experience in developing parsers and I don't
fully understand all of the more advanced features of javacc. Can you give
me a more specific suggestion to try out, or can you point me to some
sample code that deals with a similar problem? If I had an avenue to
explore I would give it another go.

On Mon, Oct 3, 2016 at 6:29 PM, Tobias notifications@github.com wrote:

Sorry. I overlooked your pr after beeing back from holidays.

What I do not like about your change is this kind of token "array
whitespace* [". I have to admit, that I do not have a proper solution at
hand. Thats the reason your pr is not merged. At the moment I do not think,
that a "correct" (in my eyes ;)) solution and the "bracket" quotation of
identifiers can coexist. Maybe one could construct a solution using
syntactic or token lookahead .. Tobias


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#306 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMgvTeHewqOQzCoamQ2A9sxAlkGF96q1ks5qwYG0gaJpZM4JL4ed
.

@opeongo
Copy link
Author

opeongo commented Dec 13, 2016

I did some more work with javacc and lexical states for another project, and learned a bit ore how to use them. I have updated my code so that it uses a lexical state change instead of the "array whitespace* [" idiom.

Unfortunately I am a bit github challenged and I was not able to merge your master in to my development tree so that I could sort out the conflicts. My changes are based on a version of your code that I checked out in the summer, and it appears to have conflicts. Sorry about that.

@opeongo opeongo closed this Dec 13, 2016
@opeongo opeongo reopened this Dec 13, 2016
@opeongo
Copy link
Author

opeongo commented Dec 13, 2016

Oops, I did not mean to close the pull request.

@opeongo
Copy link
Author

opeongo commented Jan 13, 2017

Hi Tobias

Is there anything I can do to move this along? Is is a matter of me not getting a clean pull request together, or is the solution still not right?

@wumpz
Copy link
Member

wumpz commented Jan 15, 2017

Your corrected solution looks more promising. I did not see a single test. Please add some and I will do the merge and coflict resolving. Cheers Tobias

@wumpz
Copy link
Member

wumpz commented Sep 28, 2017

This PR has come a long way.

Two points from me: first of all I merge PRs using githubs merge & squash. So if you want to control the commit you should squash it by yourself. Second, could you upgrade your PR to the actual SNAPSHOT. There were some API changes that seem to collide with your changes.

@tserr
Copy link

tserr commented Sep 29, 2017

Hi @opeongo, I tried to rebase your PR to the current SNAPSHOT (5896d83) and added the feature to use "ARRAY[...]" as PrimaryExpression (to use it as SelectItem). You find both patches attached (array-as-PrimaryExpression.patch.zip).

The file rebase-to-5896d8387bde6cd0cf822a67354954c046bf1864.patch contains your PR (based on SNAPSHOT). The file array-as-PrimaryExpression.patch contains my feature extension.

Is there anything else I can do to help getting this feature merged?

@wumpz
Copy link
Member

wumpz commented Dec 1, 2019

should work now with removal of sqlserver bracket quotation

@wumpz wumpz closed this Dec 1, 2019
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.

4 participants