Skip to content

Support reading Postgres array as JSON#1148

Merged
findepi merged 2 commits intotrinodb:masterfrom
kasiafi:PostgresArrayToJson
Oct 14, 2019
Merged

Support reading Postgres array as JSON#1148
findepi merged 2 commits intotrinodb:masterfrom
kasiafi:PostgresArrayToJson

Conversation

@kasiafi
Copy link
Copy Markdown
Member

@kasiafi kasiafi commented Jul 19, 2019

Fixes: #682

@cla-bot cla-bot bot added the cla-signed label Jul 19, 2019
@kasiafi kasiafi force-pushed the PostgresArrayToJson branch 2 times, most recently from 7b7597a to b2d7816 Compare July 19, 2019 15:21
@kasiafi
Copy link
Copy Markdown
Member Author

kasiafi commented Jul 19, 2019

Issue: #682

@kasiafi
Copy link
Copy Markdown
Member Author

kasiafi commented Jul 19, 2019

cc @guyco33

@@ -25,16 +25,19 @@
import io.prestosql.spi.connector.ConnectorSplitManager;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please extract this commit as separate PR so we could merge this without blocking on postgres changes?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done: #1181

@kasiafi kasiafi force-pushed the PostgresArrayToJson branch 2 times, most recently from 5fbaa50 to 851a264 Compare September 12, 2019 19:50
@findepi findepi changed the title Support reading Postgres array as Json Support reading Postgres array as JSON Sep 24, 2019
@kasiafi kasiafi force-pushed the PostgresArrayToJson branch from 851a264 to 14867ed Compare October 11, 2019 11:04
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

else is redundant since if clause always returns

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you should rather throw here:

throw new IllegalStateException("Unsupported array mapping type: " + getArrayMapping(session));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assign getArrayMapping(session) to a variable and reuse.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

checkArgument(jdbcArray.getClass().isArray(), "jdbcArray is not an array");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

uppercase: "JSON"

@kasiafi kasiafi force-pushed the PostgresArrayToJson branch from 14867ed to 7f8c0cb Compare October 12, 2019 16:17
@kasiafi kasiafi force-pushed the PostgresArrayToJson branch from 7f8c0cb to 1832c36 Compare October 12, 2019 16:21
@kasiafi
Copy link
Copy Markdown
Member Author

kasiafi commented Oct 12, 2019

Applied comments

@findepi findepi merged commit be5b2b4 into trinodb:master Oct 14, 2019
@findepi
Copy link
Copy Markdown
Member

findepi commented Oct 14, 2019

Merged, thanks!

@findepi findepi added this to the 321 milestone Oct 14, 2019
@findepi findepi mentioned this pull request Oct 14, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Map PostgreSQL arrays to JSON

3 participants