Remove all UNKNOWN_PROP as a type of null. #4907

xtcyclist · 2022-11-18T16:46:53Z

What type of PR is this?

bug
feature
enhancement

What problem(s) does this PR solve?

Issue(s) number:

Close https://github.com/vesoft-inc/nebula-ent/issues/1648
Close https://github.com/vesoft-inc/nebula-ent/issues/1670

Description:

UNKNOWN_PROP as a type of Null is quite troublesome. It may block normal valid queries to produce valid results.

How do you solve it?

Removed it completely from the codebase.
I think we shall not store a type of abnormality as data in a value.

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

Unit test(positive and negative cases)
Function test
Performance test
N/A

Affects:

Documentation affected (Please add the label if documentation needs to be modified.)
Incompatibility (If it breaks the compatibility, please describe it and add the label.）
If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

czpmango

LGTM.
Fwiw, are there other bad null types that need to delegate to null behavior?

yixinglu

LGTM

codecov-commenter · 2022-11-22T10:25:25Z

Codecov Report

Base: 76.77% // Head: 76.87% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (da4a402) compared to base (28f8994).
Patch coverage: 83.13% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4907      +/-   ##
==========================================
+ Coverage   76.77%   76.87%   +0.09%     
==========================================
  Files        1102     1102              
  Lines       81064    81090      +26     
==========================================
+ Hits        62240    62334      +94     
+ Misses      18824    18756      -68

Impacted Files	Coverage Δ
src/common/context/ExpressionContext.h	`100.00% <ø> (ø)`
src/common/datatypes/Value.cpp	`74.29% <ø> (+0.24%)`	⬆️
src/common/utils/DefaultValueContext.h	`0.00% <0.00%> (ø)`
src/graph/context/QueryExpressionContext.h	`100.00% <ø> (ø)`
src/graph/planner/plan/PlanNode.h	`89.42% <ø> (-0.40%)`	⬇️
src/graph/planner/plan/Query.h	`96.38% <ø> (-0.04%)`	⬇️
src/storage/context/StorageExpressionContext.h	`50.94% <ø> (-13.21%)`	⬇️
src/storage/exec/IndexEdgeScanNode.cpp	`91.75% <0.00%> (ø)`
src/storage/exec/IndexExprContext.h	`33.33% <0.00%> (-3.88%)`	⬇️
src/storage/exec/IndexVertexScanNode.cpp	`89.47% <0.00%> (ø)`
... and 61 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

This reverts commit aa62416.

This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

…vesoft-inc#5149) This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

) * Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149) This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Enhance attribute-accessing expression to ensure self-consistency Fix tck Fix parser small delete Fix tck tck fmt fix ut fix ut Fix ut Fix tck Delete v.tag.prop check Fix tck Skip some tck cases related ngdata add test case Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

abby-cyber · 2023-01-18T03:56:13Z

vesoft-inc/nebula-docs-cn#2493

) * Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149) This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Enhance attribute-accessing expression to ensure self-consistency Fix tck Fix parser small delete Fix tck tck fmt fix ut fix ut Fix ut Fix tck Delete v.tag.prop check Fix tck Skip some tck cases related ngdata add test case Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* optimize match node label (#5176) * revert strange return (#5183) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix stderr save error log (#5188) * fix processor_test timeout (#5180) * fix processor_test timeout * ... Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix error code (#5186) * rename the "test" space to "ngdata". (#5197) * rename the "test" space to "ngdata". * add ngdata * Revise the usages of FATAL, DFATAL, LOG, DLOG. (#5181) * Revise the usages of FATAL, DFATAL, LOG, DLOG. * fix. * revise dfatal. * Meta upgrade (#5174) * Meta upgrade remove all fulltext index when upgrade from V3 to V3_4 because of refacting of fulltext index * fix bug Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Fix pattern expression with same edge variable (#5192) * Fix pattern expression with same edge variable add tck fmt * add tck * Fix memory leak, remove toss gflag (#5204) * remove toss gflag * fix memory leak * loose wait job finish time Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add max_sessions_per_ip_per_user to default config file (#5207) * minor bug for adminTaskManager (#5195) * modify jobmanager ut (#5175) * modify jobmanager ut * add expired ut * avoid recover expired job * add ut * address review * move status Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add more match test cases on paths. (#5189) * improve memtracker, add missed check & remove unnecessary thenError&tryCatch check (#5199) * [memtracker] check code run with memoery check on all works refine code all code memory checked fix lint refine code & fix build with gcc+sanitize * fix build break * fix lint * refine code * remove debug code * fix test fail build with debug * fix test fail build with debug * restore commented test * minor * minor * fix bug (#5214) * fix bug * fix bug Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> * handle rpc error task status (#5212) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * chore: community badges refined (#5202) * chore: community badges refined * Update README-CN.md * Update README-CN.md * Update README-CN.md remove sifou and zhihu as aligned with the team * update linkedin URL Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> * Fix extend whtie space char. (#5213) * Fix extend whtie space char. * Format. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add lack tests of no role user. (#5196) Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> * remove memtracker DLOG (#5224) * Add tck cases for DDL (#5220) * more TCK tests for variable pattern match clause (#5215) * cleanup * same src/dst for variable length pattern * variable pattern in where clause * variable scope tests in path pattern * More tests * More tests Co-authored-by: jimingquan <mingquan.ji@vesoft.com> * Resumed the evaluation fo vertices in AttributeExpression (UTs included) (#5229) * add memtracker flags to conf (#5231) * add memtracker flags to conf * typo * refine * refine * add balance job type to filter when create backup (#5228) * add more job type to filter when create backup * log add job * add log before acquire snapshot lock Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Fix update sessions when leader change happens (#5225) * Fix udpate sessions when leader change happens * Handle errors on the graph side * Address comments * Address comments * fix match step range (#5216) * use smart pointer change raw pointer * fix error * fix test error * address comment Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Update response message when adding schema historically existed (#5227) * update the error code and message for checking history schemas * update tck * update comment * change to log error * fix ddl tck * increase wait time in schema.feature Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix error code (#5233) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * print memory stats default to false (#5234) * print memory stats default to false * update conf Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix bug of extract prop expr visitor (#5238) * forbid invalid prop expr used in cypher (#5242) * Fix mistake push down limit with skip. (#5241) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix delete fulltext index (#5239) * fix delete fulltext index * fix es delete error 1. remove get Rowreader if op is delete 2. delete es data when value is null Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Change the default value of session_reclaim_interval_secs to 60 seconds (#5246) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Enhance attribute-accessing expression to ensure self-consistency (#5230) * Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149) This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Enhance attribute-accessing expression to ensure self-consistency Fix tck Fix parser small delete Fix tck tck fmt fix ut fix ut Fix ut Fix tck Delete v.tag.prop check Fix tck Skip some tck cases related ngdata add test case Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix ft index of fixed string (#5251) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add tck test (#5253) * add allpath test * add shortest path test case * add subgraph test case * add go test case * add go test case * Add more session tests (#5256) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Revert "do not check term for leader info by default para" (#5266) This reverts commit 593bffc. * modify ft index default limit size (#5260) * modify ft index default limit size * fix test Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> * Test/yield (#5267) * Add some tests about yield. * Add more tests. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add another cert to test CA don't match. (#5247) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix baton miss reset in StorageJobExecutor (#5269) * Report errors on where clauses in optional match queries. (#5273) * [test case] Check DML cases (#5264) * Check DML cases * Add chinses char tests Add more tests Add mero delete edge tests * Revert cases * fix third party version in package.sh (#5281) The dump_syms tool path should be match with third party version. * Test/user (#5139) * Add some tests about user management. * Add tests about user roles. * Format. * Fix tck fixture name. * Fix step name. * Change step name. --------- Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix https (#5283) * fix memtracker bugs during stress test on graphd and storaged (#5276) * fix memtracker bugs during stress test on graphd and storaged * fix lint * fix RocksEngine memory leak of raw pointer iter * add ENABLE_MEMORY_TRACKER build option & support adaptive limit for MemoryTracker * delete debug log * refine log * refine log * fix build * refine error log * print warning if memtracker is off * fix rocksdb leak by turn off memcheck * refine synamic-self-adaptive * fix cmake check * minor * minor * minor * minor * minor * refine double equel compare --------- Co-authored-by: jimingquan <mingquan.ji@vesoft.com> Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com> Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com> Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> Co-authored-by: canon <87342612+caton-hpg@users.noreply.github.com> Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: hs.zhang <22708345+cangfengzhs@users.noreply.github.com> Co-authored-by: kyle.cao <kyle.cao@vesoft.com> Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com> Co-authored-by: liwenhui-soul <38217397+liwenhui-soul@users.noreply.github.com> Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com> Co-authored-by: codesigner <codesigner.huang@vesoft.com> Co-authored-by: Wey Gu <weyl.gu@gmail.com> Co-authored-by: shylock <33566796+Shylock-Hg@users.noreply.github.com> Co-authored-by: pengwei.song <90180021+pengweisong@users.noreply.github.com> Co-authored-by: haowen <19355821+wenhaocs@users.noreply.github.com> Co-authored-by: George <58841610+Shinji-IkariG@users.noreply.github.com>

…vesoft-inc#2058) This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

xtcyclist requested a review from a team as a code owner November 18, 2022 16:46

remove all UNKNOWN_PROP as a type of null.

5af8299

xtcyclist force-pushed the fix_unkown_prop branch from f8c8c65 to 5af8299 Compare November 18, 2022 17:13

xtcyclist added the ready-for-testing PR: ready for the CI test label Nov 18, 2022

xtcyclist added 3 commits November 19, 2022 11:12

fix format

74c4c59

update ut.

d1213f9

update tck

66cb454

czpmango approved these changes Nov 22, 2022

View reviewed changes

Merge branch 'master' into fix_unkown_prop

1b14ecc

yixinglu approved these changes Nov 22, 2022

View reviewed changes

Merge branch 'master' into fix_unkown_prop

e22f432

yixinglu added 2 commits November 22, 2022 19:46

Merge branch 'master' into fix_unkown_prop

da4a402

Merge branch 'master' into fix_unkown_prop

4d03ea9

czpmango approved these changes Nov 23, 2022

View reviewed changes

yixinglu approved these changes Nov 23, 2022

View reviewed changes

yixinglu requested a review from dutor November 23, 2022 08:05

Merge branch 'master' into fix_unkown_prop

f1abdfd

Sophie-Xie merged commit aa62416 into vesoft-inc:master Nov 28, 2022

HarrisChu added the doc affected PR: improvements or additions to documentation label Nov 29, 2022

wey-gu mentioned this pull request Dec 3, 2022

Weekly Report 2022-12-02 vesoft-inc/nebula-community#145

Closed

xtcyclist mentioned this pull request Dec 5, 2022

added a deprecated marker in common.thrift #4985

Merged

11 tasks

xtcyclist deleted the fix_unkown_prop branch December 5, 2022 04:34

xtcyclist added a commit to xtcyclist/nebula that referenced this pull request Dec 29, 2022

Revert "Remove all UNKNOWN_PROP as a type of null. (vesoft-inc#4907)"

17504bb

This reverts commit aa62416.

xtcyclist added a commit to xtcyclist/nebula that referenced this pull request Dec 29, 2022

Revert "Remove all UNKNOWN_PROP as a type of null. (vesoft-inc#4907)"

a4c09c2

This reverts commit aa62416.

xtcyclist added a commit to xtcyclist/nebula that referenced this pull request Dec 29, 2022

Revert "Remove all UNKNOWN_PROP as a type of null. (vesoft-inc#4907)"

07a1554

This reverts commit aa62416.

xtcyclist added a commit to xtcyclist/nebula that referenced this pull request Dec 29, 2022

Revert "Remove all UNKNOWN_PROP as a type of null. (vesoft-inc#4907)"

3d8050d

This reverts commit aa62416.

codesigner pushed a commit that referenced this pull request Dec 29, 2022

Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149)

1fae55d

This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

abby-cyber self-assigned this Jan 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove all UNKNOWN_PROP as a type of null. #4907

Remove all UNKNOWN_PROP as a type of null. #4907

xtcyclist commented Nov 18, 2022 •

edited

Loading

czpmango left a comment

yixinglu left a comment

codecov-commenter commented Nov 22, 2022 •

edited

Loading

abby-cyber commented Jan 18, 2023

Remove all UNKNOWN_PROP as a type of null. #4907

Remove all UNKNOWN_PROP as a type of null. #4907

Conversation

xtcyclist commented Nov 18, 2022 • edited Loading

What type of PR is this?

What problem(s) does this PR solve?

Issue(s) number:

Description:

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Release notes:

czpmango left a comment

Choose a reason for hiding this comment

yixinglu left a comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 22, 2022 • edited Loading

Codecov Report

abby-cyber commented Jan 18, 2023

xtcyclist commented Nov 18, 2022 •

edited

Loading

codecov-commenter commented Nov 22, 2022 •

edited

Loading