feat: flink sql add create space #66

liuxiaocs7 · 2022-07-13T16:28:23Z

Try to close #62.

codecov-commenter · 2022-07-13T16:41:34Z

Codecov Report

Merging #66 (8e4435f) into master (93df7a1) will increase coverage by 11.82%.
The diff coverage is 63.20%.

@@              Coverage Diff              @@
##             master      #66       +/-   ##
=============================================
+ Coverage     49.72%   61.54%   +11.82%     
- Complexity      190      291      +101     
=============================================
  Files            50       52        +2     
  Lines          1613     1784      +171     
  Branches        153      166       +13     
=============================================
+ Hits            802     1098      +296     
+ Misses          743      596      -147     
- Partials         68       90       +22

Impacted Files	Coverage Δ
...onnector/nebula/catalog/AbstractNebulaCatalog.java	`26.56% <33.33%> (+26.56%)`	⬆️
...ache.flink/connector/nebula/utils/NebulaSpace.java	`47.05% <47.05%> (ø)`
....flink/connector/nebula/catalog/NebulaCatalog.java	`29.29% <57.81%> (+29.29%)`	⬆️
...che.flink/connector/nebula/utils/NebulaSpaces.java	`91.30% <91.30%> (ø)`
...ink/connector/nebula/utils/NebulaCatalogUtils.java	`50.00% <100.00%> (+50.00%)`	⬆️
...e.flink/connector/nebula/utils/NebulaConstant.java	`95.00% <100.00%> (+0.55%)`	⬆️
...ache.flink/connector/nebula/utils/NebulaUtils.java	`74.60% <100.00%> (+2.67%)`	⬆️
...ebula/connection/NebulaMetaConnectionProvider.java	`55.55% <0.00%> (ø)`
...connector/nebula/table/NebulaRowDataConverter.java	`59.52% <0.00%> (ø)`
...nnector/nebula/connection/NebulaClientOptions.java	`91.25% <0.00%> (+1.25%)`	⬆️
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

liuxiaocs7 · 2022-07-14T13:07:11Z

Refer to Flink SQL doc and Nebula doc.

CREATE DATABASE [IF NOT EXISTS] [catalog_name.]db_name
  [COMMENT database_comment]
  WITH (key1=val1, key2=val2, ...)

CREATE SPACE [IF NOT EXISTS] <graph_space_name> (
    [partition_num = <partition_number>,]
    [replica_factor = <replica_number>,]
    vid_type = {FIXED_STRING(<N>) | INT[64]}
    )
    [COMMENT = '<comment>'];

I register the NebulaCatalog to flink table environment and use the catalog. Then execute create database statement will go to createDatabase method. In this method, we use graph client to execute create space ngql statement.

Test Environment:

Windows Virtual Machine(Centos7), Nebula is deployed by Docker.

And result is just as follow:

Nicole00 · 2022-08-03T08:11:06Z

connector/src/main/java/org.apache.flink/connector/nebula/catalog/NebulaCatalog.java

+     * @param b true if contains [if not exists] else false
+     */
+    @Override
+    public void createDatabase(String s, CatalogDatabase catalogDatabase, boolean b)


please use more meaningful variable name for s and b.
eg: dataBaseName, useIfNotExists

sure, these variable name is so bad. I'll change them. And should i change the variable names in the method it inherits.

In AbstractNebulaCatalog, the method definition looks like this:

@Override public void createDatabase(String s, CatalogDatabase catalogDatabase, boolean b) throws CatalogException { throw new UnsupportedOperationException(); }

great catch, the parent method has bad variable names, please change it too, thanks.

Sorry for my late reply, this week i am many other things to do, i am glad to do this as soon as possible

This 9e2c2b5 commit update the bad variable names in AbstractNebulaCatalog

Nicole00 · 2022-08-03T08:12:57Z

connector/src/main/java/org.apache.flink/connector/nebula/catalog/NebulaCatalog.java

+            LOG.debug("create space success.");
+        } else {
+            LOG.error("create space failed: {}", execResult.getErrorMessage());
+            throw new CatalogException("create space failed.");


append the error message of execResult

it means to append execResult error message to CatalogException message?

got it, e23cdfd should sovle this i think, thanks for your kindly suggestion.

Nicole00 · 2022-08-11T03:37:28Z

connector/src/main/java/org.apache.flink/connector/nebula/catalog/AbstractNebulaCatalog.java

+        checkArgument(
+                !StringUtils.isNullOrWhitespaceOnly(address),
+                "address cannot be null or empty."
+        );


thanks for the todo work

my pleasure.

Nicole00 · 2022-08-12T03:03:58Z

connector/src/main/java/org.apache.flink/connector/nebula/catalog/NebulaCatalog.java

+                !isNullOrWhitespaceOnly(dataBaseName), "space name cannot be null or empty.");
+        checkNotNull(catalogDatabase, "space cannot be null.");
+
+        if (ignoreIfExists && listDatabases().contains(dataBaseName)) {


from the view of product consistency, if ignoreIfExists is true, we should also give the positive feedback.

For Nebula, if a space exists, we'll get SUCCEED when create a repeated space, even though they have different properties, such as vid_type.

sure, there are indeed logical inconsistencies, i'll fix it.

According to the doc

Now, if the space has already existed and create the space with IF NOT EXISTS clause it will do nothing. just as
ded8300.

Test Result:

INFO [main] - Repeat to create space, db1 already exists, no effect.

Do you think it's OK to revise this way?

I think that's fine, thanks for this kindly log info.

Nicole00

LGTM

liuxiaocs7 added 2 commits July 14, 2022 00:26

feat: flink sql add create space

6ba6069

fix CI

7ee00ac

Nicole00 reviewed Aug 3, 2022

View reviewed changes

fix by review suggestion

e23cdfd

Nicole00 reviewed Aug 11, 2022

View reviewed changes

Nicole00 reviewed Aug 12, 2022

View reviewed changes

liuxiaocs7 added 3 commits August 12, 2022 13:14

refactor: bad variable names

9e2c2b5

fix: create same name space no effect

ded8300

fix: DEFAULT_REPLICA_FACTOR setting

8e4435f

Nicole00 approved these changes Aug 12, 2022

View reviewed changes

Nicole00 merged commit 677ed77 into vesoft-inc:master Aug 12, 2022

wey-gu mentioned this pull request Aug 13, 2022

Weekly Report 2022-08-12 vesoft-inc/nebula-community#127

Closed

liuxiaocs7 deleted the create_space branch September 28, 2022 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: flink sql add create space #66

feat: flink sql add create space #66

liuxiaocs7 commented Jul 13, 2022

codecov-commenter commented Jul 13, 2022 •

edited

Loading

liuxiaocs7 commented Jul 14, 2022

Nicole00 Aug 3, 2022

liuxiaocs7 Aug 3, 2022

Nicole00 Aug 4, 2022

liuxiaocs7 Aug 11, 2022

liuxiaocs7 Aug 12, 2022

Nicole00 Aug 3, 2022

liuxiaocs7 Aug 3, 2022

Nicole00 Aug 4, 2022

liuxiaocs7 Aug 11, 2022

Nicole00 Aug 11, 2022

liuxiaocs7 Aug 12, 2022

Nicole00 Aug 12, 2022

liuxiaocs7 Aug 12, 2022

liuxiaocs7 Aug 12, 2022

Nicole00 Aug 12, 2022

Nicole00 left a comment

feat: flink sql add create space #66

feat: flink sql add create space #66

Conversation

liuxiaocs7 commented Jul 13, 2022

codecov-commenter commented Jul 13, 2022 • edited Loading

Codecov Report

liuxiaocs7 commented Jul 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nicole00 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 13, 2022 •

edited

Loading