Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: support sub-column for informix #1958

Closed
wants to merge 2 commits into from
Closed

fix: support sub-column for informix #1958

wants to merge 2 commits into from

Conversation

SheldonKubor
Copy link
Contributor

try to close #1832

@manticore-projects
Copy link
Contributor

Greetings!

I need to ask: does this really belong to the Column rather then to an ArrayExpression?
We really need to avoid having 2 different interpretations of arrays.

manticore-projects
manticore-projects requested changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@manticore-projects manticore-projects left a comment

Choose a reason for hiding this comment

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

Please use ArrayConstructor instead. Those calls are 2 dimensional arrays as far as I understand it, aren't them?

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt
}
{
data = RelObjectNameList()
( LOOKAHEAD(5) "[" subColumnFirst=<S_LONG> "," subColumnLast=<S_LONG> "]" )?
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really about S_LONG only? No variables or expressions here?

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt
[ LOOKAHEAD(2) <K_COMMENT> tk=<S_CHAR_LITERAL> ]
// @todo: we better should return a SEQUENCE instead of a COLUMN
[ "." <K_NEXTVAL> { data.add("nextval"); } ]

[ LOOKAHEAD(2) arrayConstructor = ArrayConstructor(false) ]
[ LOOKAHEAD(5) arrayConstructor = ArrayConstructor(false) ]
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not convinced that this is the right logical order or interpretation. How can we still have an ArrayConstructor after having this new Column format?

@SheldonKubor
Copy link
Contributor Author

Greetings!

I need to ask: does this really belong to the Column rather then to an ArrayExpression? We really need to avoid having 2 different interpretations of arrays.

In my opinion, for informix , there is a special sub-column syntax just like col[m, n], just two sub-column, ArrayExpression can be lots of element

@manticore-projects
Copy link
Contributor

but how are those different from Arrays?
I really want to avoid an overlapping implementation here for only one exotic RDBMS.

@SheldonKubor
Copy link
Contributor Author

Contributor

OK, there's not much difference, I will close this pr. Sorry for that. I thought too much. Thanks for your time!

@SheldonKubor SheldonKubor deleted the fix-issue-1832 branch February 7, 2024 08:10
@manticore-projects
Copy link
Contributor

Sorry brother, shit can happen to the best.
Thank you big time for your kind help and support.

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

@manticore-projects manticore-projects manticore-projects requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] JSQLParser Version 4.6: Informix : Encountered unexpected token: "," ","
2 participants
@SheldonKubor @manticore-projects