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.

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

sql: bump H2 and JOOQ to latest versions and address array syntax change #1144

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

lalithsuresh
Copy link
Contributor

@lalithsuresh lalithsuresh commented Jan 25, 2022

The new H2 version uses the standard SQL syntax for arrays (that is, qualified with the type of the array).

@mbudiu-vmw I addressed most issues, but two tests still fail. They both relate to a type-confusion on the return type of array_agg(). I couldn't immediately tell if it's to do with the jooq code or whether it's on the ddlog -> sql compiler side. Please have a look, otherwise I'll dig deeper later this week.

Signed-off-by: Lalith Suresh lsuresh@vmware.com

Signed-off-by: Lalith Suresh <lsuresh@vmware.com>
@@ -64,7 +64,9 @@ protected String visitCreateTable(final CreateTable node, final String sql) {
final Pattern arrayType = Pattern.compile("ARRAY\\((.+)\\)");
Matcher m = arrayType.matcher(h2Type);
if (m.find()) {
h2Type = "array";
final String match = m.group();
final String arrayTypeStr = match.substring(6, match.length() - 1);

Choose a reason for hiding this comment

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

I think the right way to do this is to get group(1).

@mihaibudiu mihaibudiu merged commit 39cd4f4 into vmware:master Jan 28, 2022
@mihaibudiu mihaibudiu deleted the h2-upgrade branch January 28, 2022 01:11
@mihaibudiu mihaibudiu restored the h2-upgrade branch January 28, 2022 01:30
mihaibudiu pushed a commit that referenced this pull request Jan 28, 2022
mihaibudiu pushed a commit that referenced this pull request Jan 28, 2022
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.

2 participants