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

feat(sink): sink one-dimensional array data type to postgres and mysql #10032

Merged
merged 31 commits into from
Jun 7, 2023

Conversation

StrikeW
Copy link
Contributor

@StrikeW StrikeW commented May 29, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

  • Support array data type of primitive types (integer, float/double, varchar), we only support sink one-dimensional array to downstream. We can add support for more field types on demand in the future.
  • Convert array data type to a string for MySQL (ref), for example ARRAY['Value 1', 'Value 2'] will be converted to Value 1,Value 2

#9919
close #9918

Checklist For Contributors

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future. (Please refer to the issue)
  • All checks passed in ./risedev check (or alias, ./risedev c)

Checklist For Reviewers

  • I have requested macro/micro-benchmarks as this PR can affect performance substantially, and the results are shown.

Documentation

Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

  • Connector (sources & sinks)

Release note

  • Support Array data type of primitive types (integer, float/double, varchar), we only support sink one-dimensional array to downstream now.
  • For databases except for Postgres, the Array data type will be converted to a string

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has totally checked 3526 files.

Valid Invalid Ignored Fixed
1570 1 1955 0
Click to see the invalid file list
  • java/connector-node/connector-api/src/main/java/com/risingwave/connector/api/ColumnDesc.java

@StrikeW StrikeW force-pushed the siyuan/jdbc-sink-array branch 2 times, most recently from a26c062 to 4c7ba7c Compare May 29, 2023 02:22
@StrikeW StrikeW force-pushed the siyuan/jdbc-sink-array branch from 4c7ba7c to 46ad92b Compare May 29, 2023 03:33
@StrikeW StrikeW force-pushed the siyuan/jdbc-sink-array branch from 78e3776 to d98e4b4 Compare May 30, 2023 11:03
@StrikeW StrikeW changed the title (WIP) feat(sink): add array data type for postgres sink feat(sink): add array data type for postgres sink May 31, 2023
@StrikeW StrikeW added the user-facing-changes Contains changes that are visible to users label May 31, 2023
@github-actions github-actions bot removed the user-facing-changes Contains changes that are visible to users label May 31, 2023
@StrikeW StrikeW changed the title feat(sink): add array data type for postgres sink feat(sink): sink one-dimensional array data type to postgres and mysql May 31, 2023
@StrikeW StrikeW requested review from fuyufjh, tabVersion and wenym1 May 31, 2023 09:06
Copy link
Member

@fuyufjh fuyufjh left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #10032 (833a753) into main (1897aef) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main   #10032      +/-   ##
==========================================
- Coverage   70.85%   70.82%   -0.04%     
==========================================
  Files        1234     1234              
  Lines      211333   211410      +77     
==========================================
- Hits       149741   149729      -12     
- Misses      61592    61681      +89     
Flag Coverage Δ
rust 70.82% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/connector/src/sink/remote.rs 57.07% <0.00%> (ø)
src/java_binding/src/lib.rs 1.08% <0.00%> (-0.18%) ⬇️
src/meta/src/stream/source_manager.rs 46.41% <0.00%> (ø)

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@StrikeW StrikeW force-pushed the siyuan/jdbc-sink-array branch from 870329a to c1dd758 Compare June 5, 2023 05:04
@StrikeW
Copy link
Contributor Author

StrikeW commented Jun 5, 2023

Comments are fixed. PTAL again. @fuyufjh @tabVersion
I think we can leverage StringUtils.join() to convert an Array into a String. The test cases have been updated to reflect the changes.

@StrikeW StrikeW requested a review from fuyufjh June 6, 2023 06:07
Copy link
Member

@fuyufjh fuyufjh left a comment

Choose a reason for hiding this comment

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

LGTM

@fuyufjh
Copy link
Member

fuyufjh commented Jun 6, 2023

Comments are fixed. PTAL again. @fuyufjh @tabVersion I think we can leverage StringUtils.join() to convert an Array into a String. The test cases have been updated to reflect the changes.

Acceptable to me

@StrikeW StrikeW added this pull request to the merge queue Jun 7, 2023
Merged via the queue into main with commit 3d89b39 Jun 7, 2023
@StrikeW StrikeW deleted the siyuan/jdbc-sink-array branch June 7, 2023 02:29
@hengm3467 hengm3467 added the 📖✓ Covered or will be covered in the user docs. label Jun 9, 2023
lmatz added a commit that referenced this pull request Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature user-facing-changes Contains changes that are visible to users 📖✓ Covered or will be covered in the user docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: add data types testing for postgres sink
4 participants