LGTM alert fixes #1383
Merged
LGTM alert fixes #1383
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Comment on lines
+68
to
+69
| unsigned long array_size = cap * elem_sz; | ||
| array_hdr_t *hdr = (array_hdr_t *)array_alloc_fn(sizeof(array_hdr_t) + array_size); |
There was a problem hiding this comment.
Can this be fixed with an inlined cast instead of a separate variable? I think that would be preferable.
|
This pull request fixes 11 alerts when merging a115310 into 38f6f56 - view on LGTM.com fixed alerts:
|
swilly22
suggested changes
Oct 16, 2020
| @@ -35,7 +35,7 @@ static void *_GraphContextType_RdbLoad(RedisModuleIO *rdb, int encver) { | |||
| REDISGRAPH_MODULE_VERSION, encver); | |||
| return NULL; | |||
| // Previous version. | |||
| } else if(encver >= GRAPHCONTEXT_TYPE_DECODE_MIN_V && encver < GRAPH_ENCODING_VERSION_LATEST) { | |||
There was a problem hiding this comment.
What's the issue here? why aren't we validating the minimum version?
There was a problem hiding this comment.
In line 33 we check if(encver < GRAPHCONTEXT_TYPE_DECODE_MIN_V).
So if we reached line 38, the condition of line 33 must be false. Therefore checking the if encver >= GRAPHCONTEXT_TYPE_DECODE_MIN_V in line 38 is redundant.
swilly22
approved these changes
Oct 18, 2020
|
This pull request fixes 11 alerts when merging 23a4a2d into 38f6f56 - view on LGTM.com fixed alerts:
|
swilly22
added a commit
that referenced
this pull request
Nov 8, 2020
* added init time NodeScanCtx update on label scan (#1358) * added init time NodeScanCtx update on label scan * rephrase comment Co-authored-by: Roi Lipman <[email protected]> * Bfs procedure call (#1306) * Import LAGraph files * Apply autoformatter * Add LAGraph include file * WIP Add BFS procedure * tmp Utility * Revert LAGraph inclusions * Replace LAGraph reference with GraphBLAS references * WIP * WIP * Add BFSTree procedure * Add unit tests * Update documentation * Resolve compiler warnings * Address PR comments * Make singular BFS procedure with variable outputs * Remove start_node yield * Address PR comments * Remove yield-edges BFS input arg * Address PR comments * Switch to LAGraph_bfs_pushpull * Remove unused code paths from LAGraph BFS algorithm * procedure invocation receives yield array * consume BFS result using iterator * test BFS returning no results * clean up, address PR comments Co-authored-by: swilly22 <[email protected]> Co-authored-by: Roi Lipman <[email protected]> * Separate ExecutionPlan op construction logic into different files (#1352) * Separate ExecutionPlan op construction logic into different files * Address PR comments * Refactor header files * Address PR comments Co-authored-by: Roi Lipman <[email protected]> * Support array properties in bulk loader (#1365) * Support array properties in bulk loader * Address PR comments * Add no_multi_key & hash_policy (#1363) * Add no_multi_key & hash_policy * Update ramp.yml At the moment we can't handle user hash policy due to virtual keys Co-authored-by: Roi Lipman <[email protected]> * Add LGTM (#1367) * read only query (#1364) * read only query * added sleep, try to avoid ci failure * added more write scenarios. added replica shutdown * added skip flag to avoid memcheck failure * fixed PR comments * Update module.c * try to skip on any debugger. added more write scenarios * checkSlaveSynced moved to use time.sleep. Another try to skip debuger * added RO_QUERY to commands Co-authored-by: Roi Lipman <[email protected]> * Fix typo (#1368) Co-authored-by: Roi Lipman <[email protected]> * Add GRAPH.BULK documentation and link to bulk loader documentation (#1371) * Add GRAPH.BULK documentation and link to bulk loader documentation * Address PR comments * Allow upper case letters in proc names (#1372) * added to_lower for procedure name in Proc_get to support both upper and lower cases * new test for lookup_case_insensitive * Created strutil and moved the implementations of str_tolower and str_toupper to it. Remove the static implementations of these functions and replaced every call with a new call for str_tolower. minor changes in getProc and procRegister due to PR * fix indentations * fix indentations * minor changes * minor changes * indentations fix * indentations fix * indentations fix * indentations fix * indentations fix * PR fixes * PR fixes * PR fixes * PR fixes * PR fixes * PR fixes * PR fixes * PR fixes Co-authored-by: Roi Lipman <[email protected]> * Adding a new procedure that returns all the procedures in the system (#1377) * Adding a new procedure that returns all the procedures in the system. This procedure may yield up to two fields for each procedure: its name and/or its mode (READ/WRITE). * PR fixes * Saving raxIterator reference instead of allocating it on the heap in proc_proceudres_private_data. More PR code fixes * Remove the getter "Procedure_ReadOnly" that gets only the procedures name, leaving only the getter "Procedure_IsReadOnly" that gets the whole procedure object. * Always set GraphCtx in QueryCtx while decoding graph keys (#1381) * LGTM alert fixes (#1383) Co-authored-by: Roi Lipman <[email protected]> * Avoid double free of heap-allocated values in CASE...WHEN statements (#1386) * Avoid double free of heap-allocated values in CASE...WHEN statements * Update test_function_calls.py Co-authored-by: Roi Lipman <[email protected]> * With filter scoping (#1361) * Don't migrate WITH filters into Merge and Apply scopes * Don't place filters in previous scope for filtered aliases * Fix memory error * remove vector * consolidate operation locating * execution plan construction refactoring WIP * move arithmetic expression construction from AST to AR_EXP * discover skip, limit and black-listed ops modifiers * PR fixes * Fixes continued * Fix ResultSet updates in UNION queries * Clarify comments, minor cleanup * Fix unit tests * Fix caching logic for SKIP and LIMIT * Address PR comments * tidy up execution plan construction and join op * reset limit on aggregation * define 16 as the default batch size for traversal operations Co-authored-by: swilly22 <[email protected]> Co-authored-by: Roi Lipman <[email protected]> * Updates to Astyle config file (#1384) Co-authored-by: Roi Lipman <[email protected]> * replace gitter with Discord (#1395) * reaplace gitter with Discord * replace gitter to discord * Allow property accesses on non-identifier entity references (#1389) * Allow property accesses on non-identifier entity references * access entity attribute only via the property function * address PR comments Co-authored-by: swilly22 <[email protected]> Co-authored-by: Roi Lipman <[email protected]> * Minor suggested update. (#1398) * replace automation service (#1399) * redundent filter construction and placement (#1397) * redundent filter construction and placement * Add validating flow tests Co-authored-by: Jeffrey Lovitz <[email protected]> * Parse full argv array on main thread (#1298) * Parse full argv array on main thread * remove read_flags from cmd_query Co-authored-by: Roi Lipman <[email protected]> Co-authored-by: swilly22 <[email protected]> * query validation should not check if procedure outputs been defined (#1406) * query validation should not check if procedure outputs been defined * address PR comments * Add trusted-by logos (#1412) * Add trusted-by logos * add more logos * Fuzzy test crashes (#1408) * Correctly place filters without referenced variables * Emit compile-time error on constant non-boolean filter * Validate references per clause instead of per scope * Emit error on failed filter placement in optimize_cartesian_product * Fix free of invalid value in OpUnwind * Handle NULL sources in variable-length traversals * Address PR comments * Post-rebase fixes * Address PR comments * introduce AST unsupported op error Co-authored-by: swilly22 <[email protected]> * Graph version (#1382) * add graph version to graph context object * change graph version from string to uint32 * testing graph version * response with an error when graph version mismatch * release graph context on version mismatch * address PR comments * increase command arity to accomadate graph version argument * address PR comments Co-authored-by: DvirDukhan <[email protected]> * Release version 2.2.8. Merged from master. Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: Roi Lipman <[email protected]> Co-authored-by: Jeffrey Lovitz <[email protected]> Co-authored-by: swilly22 <[email protected]> Co-authored-by: Guy Korland <[email protected]> Co-authored-by: Yahya <[email protected]> Co-authored-by: Simon Prickett <[email protected]>
pnxguide
pushed a commit
to CMU-SPEED/RedisGraph
that referenced
this pull request
Mar 22, 2023
Co-authored-by: Roi Lipman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some small fixes due to LGTM alerts