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

fix: Char covert to String on remove specified unicode #1664

Merged
merged 4 commits into from
Dec 1, 2021

Conversation

coderzc
Copy link
Member

@coderzc coderzc commented Nov 22, 2021

fixed #1638

@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #1664 (5189258) into master (5bd37d8) will increase coverage by 0.40%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1664      +/-   ##
============================================
+ Coverage     66.56%   66.97%   +0.40%     
- Complexity     7039     7068      +29     
============================================
  Files           421      421              
  Lines         34673    34675       +2     
  Branches       4803     4803              
============================================
+ Hits          23081    23223     +142     
+ Misses         9247     9107     -140     
  Partials       2345     2345              
Impacted Files Coverage Δ
.../baidu/hugegraph/backend/query/ConditionQuery.java 85.94% <100.00%> (+0.15%) ⬆️
...du/hugegraph/backend/tx/GraphIndexTransaction.java 83.81% <100.00%> (-0.04%) ⬇️
...c/main/java/com/baidu/hugegraph/task/HugeTask.java 72.30% <0.00%> (+0.30%) ⬆️
...va/com/baidu/hugegraph/task/ServerInfoManager.java 71.34% <0.00%> (+0.56%) ⬆️
...hugegraph/backend/query/ConditionQueryFlatten.java 75.63% <0.00%> (+0.84%) ⬆️
.../java/com/baidu/hugegraph/auth/RolePermission.java 91.76% <0.00%> (+1.17%) ⬆️
...in/java/com/baidu/hugegraph/auth/HugeResource.java 79.22% <0.00%> (+1.29%) ⬆️
...baidu/hugegraph/backend/store/mysql/MysqlUtil.java 96.61% <0.00%> (+3.38%) ⬆️
...va/com/baidu/hugegraph/auth/HugeAuthenticator.java 40.21% <0.00%> (+5.43%) ⬆️
...aidu/hugegraph/backend/store/mysql/MysqlTable.java 81.59% <0.00%> (+5.76%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bd37d8...5189258. Read the comment docs.

javeme
javeme previously approved these changes Nov 23, 2021
}

// Ignore unicode \u0000 to \u0003
words.removeAll(IGNORE_SYM_SET);
Copy link
Contributor

Choose a reason for hiding this comment

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

can add a test case?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@@ -8582,6 +8582,29 @@ public void testQueryByHasIdEmptyListInPage() {
Assert.assertNull(page);
}

@Test
public void testIgnoreSymbol() {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to test testQueryBySearchIndexWithSpecialSymbol

@@ -103,6 +103,15 @@
public static final String INDEX_SYM_NULL = "\u0001";
public static final String INDEX_SYM_EMPTY = "\u0002";
public static final char INDEX_SYM_MAX = '\u0003';
public static final Set<String> IGNORE_SYM_SET;
Copy link
Contributor

Choose a reason for hiding this comment

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

add fixed #1638 to commit message

.by("name").search().ifNotExist().create();

Vertex vertex = graph.addVertex(T.label, "person", "name",
"xyz\u0000abc", "city", "Hongkong",
Copy link
Contributor

Choose a reason for hiding this comment

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

also test \u0003


Vertex vertex = graph.addVertex(T.label, "person", "name",
"xyz\u0000abc", "city", "Hongkong",
"age", 15);
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the behavior if adding a vertex with "name": "\u0000"

javeme
javeme previously approved these changes Nov 23, 2021
@javeme
Copy link
Contributor

javeme commented Nov 23, 2021

LGTM

imbajin
imbajin previously approved these changes Nov 23, 2021
@coderzc
Copy link
Member Author

coderzc commented Nov 23, 2021

postgresql ci failed

Error:  testQueryBySearchIndexWithSpecialSymbol(com.baidu.hugegraph.core.VertexCoreTest)  Time elapsed: 0.304 s  <<< ERROR!
com.baidu.hugegraph.backend.BackendException: Failed to commit
	at com.baidu.hugegraph.core.VertexCoreTest.testQueryBySearchIndexWithSpecialSymbol(VertexCoreTest.java:8604)
Caused by: java.sql.BatchUpdateException: Batch entry 0 INSERT INTO g_ai (FIELD_VALUES, INDEX_LABEL_ID, ELEMENT_IDS, EXPIRED_TIME) VALUES ('xyz', 276, 'S1454:xyzS1454:xyzabc', 0) ON CONFLICT (FIELD_VALUES, INDEX_LABEL_ID, ELEMENT_IDS) DO UPDATE SET FIELD_VALUES = 'xyz', INDEX_LABEL_ID = 276, ELEMENT_IDS = 'S1454:xyzS1454:xyzabc', EXPIRED_TIME = 0 was aborted: ERROR: invalid byte sequence for encoding "UTF8": 0x00  Call getNextException to see other errors in the batch.
	at com.baidu.hugegraph.core.VertexCoreTest.testQueryBySearchIndexWithSpecialSymbol(VertexCoreTest.java:8604)
Caused by: org.postgresql.util.PSQLException: ERROR: invalid byte sequence for encoding "UTF8": 0x00
	at com.baidu.hugegraph.core.VertexCoreTest.testQueryBySearchIndexWithSpecialSymbol(VertexCoreTest.java:8604)

@coderzc coderzc dismissed stale reviews from imbajin and javeme via 048751e November 24, 2021 03:12
@coderzc coderzc requested a review from javeme November 24, 2021 05:55
@coderzc coderzc force-pushed the fix_segment_words branch 2 times, most recently from e21a453 to e5cc8c1 Compare November 24, 2021 07:55
@@ -8590,10 +8590,10 @@ public void testQueryBySearchIndexWithSpecialSymbol() {
.by("name").search().ifNotExist().create();

Vertex vertex = graph.addVertex(T.label, "person", "name",
"xyz\u0000abc", "city", "Hongkong",
"xyz\u0002abc", "city", "Hongkong",
Copy link
Contributor

Choose a reason for hiding this comment

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

also add case for "xyz\u0001abc"

"age", 15);
Vertex vertex2 = graph.addVertex(T.label, "person", "name",
"\u0000", "city", "Hongkong",
"\u0002", "city", "Hongkong",
Copy link
Contributor

Choose a reason for hiding this comment

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

also add case for "\u0001"

@coderzc
Copy link
Member Author

coderzc commented Nov 24, 2021

ci failed

Error:  Tests run: 714, Failures: 0, Errors: 1, Skipped: 13, Time elapsed: 265.664 s <<< FAILURE! - in com.baidu.hugegraph.core.CoreTestSuite
Error:  testQueryBySearchIndexWithSpecialSymbol(com.baidu.hugegraph.core.VertexCoreTest)  Time elapsed: 0.151 s  <<< ERROR!
java.lang.IllegalArgumentException: Illegal value of index property: '�'
	at com.baidu.hugegraph.core.VertexCoreTest.testQueryBySearchIndexWithSpecialSymbol(VertexCoreTest.java:8610)

@javeme
Copy link
Contributor

javeme commented Nov 26, 2021

need to rebase master

@@ -61,6 +61,15 @@
public static final String INDEX_VALUE_NULL = new String("<null>");
public static final String INDEX_VALUE_EMPTY = new String("<empty>");

public static final Set<String> IGNORE_SYM_SET;
static {
List<String> list = new ArrayList<>(INDEX_SYM_MIN - INDEX_SYM_MAX);
Copy link
Contributor

Choose a reason for hiding this comment

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

error Illegal Capacity: -3, expect INDEX_SYM_MAX - INDEX_SYM_MIN

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@coderzc coderzc merged commit 83180fb into master Dec 1, 2021
@javeme javeme deleted the fix_segment_words branch December 28, 2021 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception while inserting special char values like '\\u0000'
3 participants