Adding Array Support including Unit Tests #81

akuzin1 · 2023-05-23T20:45:10Z

Description

Even though there is no dedicated array field type in OpenSearch, one can still pass an array of values into any field. However, currently there is no way to take advantage of that feature. Therefore, I added Array support to the driver, so that the user can call getArray(<column-index>) to retrieve the array object stored as a field from the returned ResultSet.

Issues Resolved

[List any issues this PR will resolve]

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Yury-Fridlyand

Few small comments. I'm going to test this manually to verify.
For documentation purposes can you add to PR description screenshots how it looks with and without this fix? Please add you sample data and mapping to let others verify the test.
Thanks

src/main/java/org/opensearch/jdbc/ArrayImpl.java

src/main/java/org/opensearch/jdbc/ResultSetImpl.java

src/main/java/org/opensearch/jdbc/types/ArrayType.java

Yury-Fridlyand · 2023-06-02T02:59:55Z

src/main/java/org/opensearch/jdbc/types/OpenSearchType.java

@@ -61,15 +63,17 @@ public enum OpenSearchType {
    STRING(JDBCType.VARCHAR, String.class, Integer.MAX_VALUE, 0, false),
    IP(JDBCType.VARCHAR, String.class, 15, 0, false),
    NESTED(JDBCType.STRUCT, null, 0, 0, false),


Maybe nested should be mapped to array

Could you explain why? I may not be following.

Nested is mapped to ARRAY in sql plugin, curious why it is different here?
OpenSearchDataType.java#L39

The sql plugin behavior always returns only the first element, doesn't actually treat it as an Array...

A bug of workbench?

…lity Signed-off-by: Andrey Kuzin <[email protected]>

akuzin1 · 2023-06-02T17:20:40Z

The sample data I used to populate OpenSearch Index:

POST sample_array_data/_doc
{
  "int_array": [1, 2, 3],
  "string_array": ["abc", "cde", "fgh"],
  "double_array": [1.25, 5.99, 12.32],
  "boolean_array": [true, false, true]
}

POST sample_array_data/_doc
{
  "int_array": [4, 5, 6],
  "string_array": ["qrs", "tuv", "xyz"],
  "double_array": [7.23, 10.50, 99.99],
  "boolean_array": [false, true, false]
}

Before:

java.lang.RuntimeException: java.sql.SQLFeatureNotSupportedException: Array is not supported
	at com.amazonaws.athena.connector.lambda.data.S3BlockSpiller.writeRows(S3BlockSpiller.java:196)
	at com.amazonaws.athena.connectors.jdbc.manager.JdbcRecordHandler.readWithConstraint(JdbcRecordHandler.java:185)
	at com.amazonaws.athena.connector.lambda.handlers.RecordHandler.doReadRecords(RecordHandler.java:197)
	at com.amazonaws.athena.connector.lambda.handlers.RecordHandler.doHandleRequest(RecordHandler.java:163)
	at com.amazonaws.athena.connector.lambda.handlers.CompositeHandler.handleRequest(CompositeHandler.java:147)
	at com.amazonaws.athena.connector.lambda.handlers.CompositeHandler.handleRequest(CompositeHandler.java:112)
Caused by: java.sql.SQLFeatureNotSupportedException: Array is not supported
	at org.opensearch.jdbc.ResultSetImpl.getArray(ResultSetImpl.java:1053)
	at com.amazonaws.athena.connectors.jdbc.manager.JdbcRecordHandler.lambda$makeFactory$1(JdbcRecordHandler.java:206)
	at com.amazonaws.athena.connector.lambda.data.writers.GeneratedRowWriter.writeRow(GeneratedRowWriter.java:119)
	at com.amazonaws.athena.connectors.jdbc.manager.JdbcRecordHandler.lambda$readWithConstraint$0(JdbcRecordHandler.java:185)
	at com.amazonaws.athena.connector.lambda.data.S3BlockSpiller.writeRows(S3BlockSpiller.java:193)
	... 5 more

After:

src/main/java/org/opensearch/jdbc/types/ArrayType.java

Yury-Fridlyand · 2023-06-02T18:30:19Z

src/main/java/org/opensearch/jdbc/types/OpenSearchType.java

@@ -61,15 +63,17 @@ public enum OpenSearchType {
    STRING(JDBCType.VARCHAR, String.class, Integer.MAX_VALUE, 0, false),
    IP(JDBCType.VARCHAR, String.class, 15, 0, false),
    NESTED(JDBCType.STRUCT, null, 0, 0, false),


A bug of workbench?

Co-authored-by: Yury-Fridlyand <[email protected]> Signed-off-by: akuzin1 <[email protected]>

Yury-Fridlyand · 2023-06-08T17:27:18Z

I suspended this PR, because it is blocked by opensearch-project/sql#1300. We need to implement this feature in SQL plugin prior to make any changes in JDBC.

Signed-off-by: Yury-Fridlyand <[email protected]>

seankao-az · 2025-04-01T17:51:28Z

Hi @akuzin1,

I noticed this draft PR has been open for a while without recent activity. I'm doing some repository maintenance and wanted to check if you're still interested in completing this work?

If you plan to continue working on this:

Please let us know your timeline
Update the PR with the latest main branch
Let us know if you need any help or guidance

If you no longer plan to work on this:

Feel free to close this PR
Or let us know and we can close it

If we don't hear back within 2 weeks, we'll close this PR to keep our PR list clean, but feel free to reopen it when you're ready to continue.

Thank you for your contribution!

akuzin1 · 2025-04-14T16:14:42Z

Hi, @seankao-az, the PR was iniatially "suspended, because it is blocked by opensearch-project/sql#1300. A feature needed to be implemented in the SQL plugin prior to making any changes in JDBC."
It appears that the issue has since been resolved and it'd be great if it can get reviewed once more? In which case I can address any comments and rebase as necessary. Thank you!

Swiddis · 2025-04-15T18:19:43Z

Don't see any clear issues and it works locally, lgtm

akuzin1 requested review from pjfitzgibbons, ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, rupal-bq, mengweieric, vamsimanohar, Swiddis, penghuo, seankao-az, MaxKsyunz and Yury-Fridlyand as code owners May 23, 2023 20:45

Yury-Fridlyand reviewed Jun 2, 2023

View reviewed changes

Added Array Support to driver as well as Unit Tests for new functiona…

a6780b5

…lity Signed-off-by: Andrey Kuzin <[email protected]>

akuzin1 force-pushed the ArraySupport branch from e15de5e to a6780b5 Compare June 2, 2023 14:57

Yury-Fridlyand reviewed Jun 2, 2023

View reviewed changes

Update src/main/java/org/opensearch/jdbc/types/ArrayType.java

9ff2b30

Co-authored-by: Yury-Fridlyand <[email protected]> Signed-off-by: akuzin1 <[email protected]>

Yury-Fridlyand marked this pull request as draft June 7, 2023 20:25

Yury-Fridlyand added a commit to Bit-Quill/sql-jdbc that referenced this pull request Jun 8, 2023

Remove opensearch-project#81.

ae22a46

Signed-off-by: Yury-Fridlyand <[email protected]>

seankao-az marked this pull request as ready for review April 15, 2025 18:01

seankao-az requested review from anirudha, acarbonetto and GumpacG as code owners April 15, 2025 18:01

Swiddis approved these changes Apr 15, 2025

View reviewed changes

Swiddis merged commit 9746f56 into opensearch-project:main Apr 15, 2025

Adding Array Support including Unit Tests #81

Adding Array Support including Unit Tests #81

Uh oh!

Conversation

akuzin1 commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yury-Fridlyand Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

akuzin1 Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

forestmvey Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

akuzin1 Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

akuzin1 commented Jun 2, 2023

Uh oh!

Uh oh!

Yury-Fridlyand Jun 2, 2023

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand commented Jun 8, 2023

Uh oh!

seankao-az commented Apr 1, 2025

Uh oh!

akuzin1 commented Apr 14, 2025

Uh oh!

Swiddis commented Apr 15, 2025

Uh oh!

Uh oh!

akuzin1 commented May 23, 2023 •

edited

Loading