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&refact:add jdbc date type support&Synchronized editorconfig #648

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

MuLeiSY2021
Copy link

Purpose of the PR

add jdbc date type support include DATE、TIME、 DATETIME、 TIMESTAMP and YEAR

  • Add a test ,which show all different type import by jdbc

Main Changes

  • I change the logic of if value is date,KafkaSource can't instaceof FileSource,So I turned them into juxtaposition logic,and add JDBC source convert
  • In mysql cases,there are many type of date,and if just type casting,the error gonna happend,so I need new var to solve this problem

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Need tests and can be verified as follows:
    • testJdbcSqlDateConvert

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jan 3, 2025
@github-actions github-actions bot added the loader hugegraph-loader label Jan 3, 2025
@dosubot dosubot bot added the enhancement New feature or request label Jan 3, 2025
@imbajin
Copy link
Member

imbajin commented Jan 3, 2025

@MuLeiSY2021
Copy link
Author

@imbajin

Thanks a lot for your contribution, could refer the doc to format the code: (to avoid a lot of unexpected changes)

Thank you for the reminder. After I checked my code style, I'm pretty sure I have already opened that configuration.
I think there might be a bug:
When I pasted the .editorconfig from the "apache/incubator-hugegraph" repository, it worked as expected, just like the original file.
However, the .editorconfig from the "apache/incubator-hugegraph-toolchain" repository didn't work.
Maybe we should migrate the .editorconfig from "apache/incubator-hugegraph" to "apache/incubator-hugegraph-toolchain" to solve this problem.

Also, I noticed that:

# Ignore the IDEA unsupported warning & it works well (indeed) 
continuation_indent_size = 16

It doesn't work on my IDEA version: "IntelliJ IDEA 2024.3.1 (Ultimate Edition)".
After referring to this article

https://youtrack.jetbrains.com/issue/IJPL-148288/Continuation-indent-size-from-IDE-setting-is-not-used-when-using-.editorconfig
@thorsten.schoening:
Thanks for reporting this issue. It seems we should use the continuations setting from the IDE's code style setting for your case.
As a workaround, you could add ij_continuation_indent_size = 8 in .editorconfig. to fix this issue.

I changed "continuation_indent_size" to "ij_continuation_indent_size", and it worked well,maybe i should ask another issue

@MuLeiSY2021 MuLeiSY2021 force-pushed the bugfix/jdbc_adapt_string branch from a7ebedc to 00b5cc5 Compare January 6, 2025 01:48
@MuLeiSY2021 MuLeiSY2021 force-pushed the bugfix/jdbc_adapt_string branch 2 times, most recently from b08e9c0 to fcb987a Compare January 6, 2025 01:53
@MuLeiSY2021 MuLeiSY2021 force-pushed the bugfix/jdbc_adapt_string branch from fcb987a to 843e513 Compare January 6, 2025 01:58
@imbajin
Copy link
Member

imbajin commented Jan 6, 2025

@imbajin

Thanks a lot for your contribution, could refer the doc to format the code: (to avoid a lot of unexpected changes)

Thank you for the reminder. After I checked my code style, I'm pretty sure I have already opened that configuration. I think there might be a bug: When I pasted the .editorconfig from the "apache/incubator-hugegraph" repository, it worked as expected, just like the original file. However, the .editorconfig from the "apache/incubator-hugegraph-toolchain" repository didn't work. Maybe we should migrate the .editorconfig from "apache/incubator-hugegraph" to "apache/incubator-hugegraph-toolchain" to solve this problem.

Also, I noticed that:

# Ignore the IDEA unsupported warning & it works well (indeed) 
continuation_indent_size = 16

It doesn't work on my IDEA version: "IntelliJ IDEA 2024.3.1 (Ultimate Edition)". After referring to this article

youtrack.jetbrains.com/issue/IJPL-148288/Continuation-indent-size-from-IDE-setting-is-not-used-when-using-.editorconfig
@thorsten.schoening:
Thanks for reporting this issue. It seems we should use the continuations setting from the IDE's code style setting for your case.
As a workaround, you could add ij_continuation_indent_size = 8 in .editorconfig. to fix this issue.

I changed "continuation_indent_size" to "ij_continuation_indent_size", and it worked well,maybe i should ask another issue

Thanks for the feedback, we also had separate configuration options for IDEA before (The config file in toolchain is outdated now, we need to update it)

@MuLeiSY2021 Ur right, we should sync the config file with the main repo https://github.com/apache/incubator-hugegraph/blob/master/.editorconfig, u could sync it in this PR if needed (And reformat your code again, thx)

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jan 6, 2025
@github-actions github-actions bot added hubble hugegraph-hubble client hugegraph-client tools hugegraph-tools spark labels Jan 6, 2025
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 50.57471% with 43 lines in your changes missing coverage. Please review.

Project coverage is 62.50%. Comparing base (b066b80) to head (0c4bf37).
Report is 45 commits behind head on master.

Files with missing lines Patch % Lines
...e/hugegraph/serializer/direct/struct/DataType.java 0.00% 14 Missing ⚠️
...e/hugegraph/serializer/direct/HBaseSerializer.java 0.00% 5 Missing ⚠️
.../hugegraph/serializer/direct/util/IdGenerator.java 0.00% 5 Missing ⚠️
...rg/apache/hugegraph/serializer/direct/util/Id.java 0.00% 3 Missing ⚠️
...ph/serializer/direct/util/SplicingIdGenerator.java 0.00% 3 Missing ⚠️
...rg/apache/hugegraph/exception/ServerException.java 77.77% 2 Missing ⚠️
...e/hugegraph/serializer/direct/struct/HugeType.java 0.00% 2 Missing ⚠️
...gegraph/serializer/direct/util/StringEncoding.java 0.00% 2 Missing ⚠️
...g/apache/hugegraph/structure/gremlin/Response.java 50.00% 2 Missing ⚠️
...nt/src/main/java/org/apache/hugegraph/api/API.java 80.00% 0 Missing and 1 partial ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff             @@
##             master     #648     +/-   ##
===========================================
  Coverage     62.49%   62.50%             
+ Complexity     1903     1004    -899     
===========================================
  Files           262      175     -87     
  Lines          9541     5243   -4298     
  Branches        886      366    -520     
===========================================
- Hits           5963     3277   -2686     
+ Misses         3190     1775   -1415     
+ Partials        388      191    -197     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MuLeiSY2021 MuLeiSY2021 force-pushed the bugfix/jdbc_adapt_string branch from 0c4bf37 to 66d4480 Compare January 7, 2025 06:11
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Jan 7, 2025
@github-actions github-actions bot removed hubble hugegraph-hubble client hugegraph-client tools hugegraph-tools spark labels Jan 7, 2025
@MuLeiSY2021 MuLeiSY2021 changed the title feat:add jdbc date type support feat&refact:add jdbc date type support&Synchronized editorconfig Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request loader hugegraph-loader size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants