Skip to content

Google Spanner support #1415

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

Closed
wants to merge 1 commit into from
Closed

Google Spanner support #1415

wants to merge 1 commit into from

Conversation

s13o
Copy link

@s13o s13o commented Nov 11, 2021

add support of a Google Spanner Interleaved table, commit timestamp options and array types

@s13o
Copy link
Author

s13o commented Nov 18, 2021

Any comments, suggestions, confusions so far?

@manticore-projects
Copy link
Contributor

Greetings,

thank you for sending this PR, it looks good to me. But only the owner of the software @wumpz will be able to merge it and he is very busy at the moment. Please have patience, thank you!

@manticore-projects
Copy link
Contributor

Greetings, you would need to resolve the conflicts and re-submit please.
(Just in case: Pull/Merger from MASTER into your repository, resolve conflicts, commit and push into your GitHub).

@@ -186,18 +188,21 @@ public String toString() {
sql += ")";
}
String options = PlainSelect.getStringList(tableOptionsStrings, false, false);
if (options != null && options.length() > 0) {
if (!options.isEmpty()) {
Copy link
Contributor

@manticore-projects manticore-projects Nov 29, 2021

Choose a reason for hiding this comment

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

Is this really NULL safe? In JSQLParser, we have (unfortunately) a lot of Lists which can be NULL.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is a StringBuilder result from the PlainSelect#getStringList(...)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thanks for checking.
isEmpty is available on Java 8? I recall it was available in Java 11 only?

Copy link
Author

@s13o s13o Nov 29, 2021

Choose a reason for hiding this comment

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

yes it is available, since 1.6
Maven build would fail otherwise but it was passed fine

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, sorry for bothering you.

Thanks for the PR, it looks very good in my opinion, well done.
But only @wumpz can merge it eventually. Please be patient.

Copy link
Author

Choose a reason for hiding this comment

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

Any chance to have it merged this year?

Copy link
Author

Choose a reason for hiding this comment

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

Is the project abandoned?

Copy link
Contributor

Choose a reason for hiding this comment

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

The project is very active and as accepted a few PRs since your contribution.
Thus you will need to resolve conflicts first (Pull + Merge + Resolve Conflicts + Commit + Push), before your PR can be accepted.

Thank you for understanding.

@wumpz
Copy link
Member

wumpz commented Jun 28, 2022

@manticore-projects Is this still active?

manticore-projects added a commit to manticore-projects/JSqlParser that referenced this pull request Dec 9, 2022
Replaces PR JSQLParser#1415, all credit goes to @s13o
Re-arranged some recently added Tokens in alphabetical order
Update Keywords
@manticore-projects
Copy link
Contributor

Salvaged into PR #1676.

wumpz pushed a commit that referenced this pull request Dec 22, 2022
* support clickhouse global keyword in join

* fix: add missing public Getter

Add public Getter for `updateSets`
Fixes #1630

* feat: Clickhouse GLOBAL JOIN

All credits to @julianzlzhang

fixes #1615
fixes #1535

* feat: IF/ELSE statements supports Block

Make `If... Else...` statements work with Blocks
Make `Statement()` production work with `Block()`
Rewrite the `Block()` related Unit Tests

fixes #1682

* fix: Revert unintended changes to the Special Oracle Tests

* fix: `SET` statement supports `UserVariable`

Make `SetStatement` parse Objects instead of Names only
Add Grammar to accept `UserVariable` (e.g. "set @Flag = 1")
Add Test Case for `UserVariable`

fixes #1682

* feat: Google Spanner Support

Replaces PR #1415, all credit goes to @s13o
Re-arranged some recently added Tokens in alphabetical order
Update Keywords

* fix: fix JSonExpression, accept Expressions

Make JSonExpression accept Expressions
Add Testcase
Expose Idents() and Operators()
Fixes #1696

* test: add Test for Issue #1237

Co-authored-by: Zhang Zhongliang <zhangzhongliang@xiaomi.com>
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.

3 participants