Skip to content

Fix toInteger() and toIntegerOrNull() don't convert Booleans #2797

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

Merged
merged 5 commits into from
Dec 29, 2022

Conversation

nafraf
Copy link
Contributor

@nafraf nafraf commented Dec 27, 2022

This PR is to fix bug #2794
The function AR_TOINTEGER() was modified to include the conversion of T_BOOL types.
Tests for toInteger() and toIntegerOrNull()were updated too.

The tck test [8] `toInteger()` failing on invalid arguments was skipped because bool type is invalid for it.
But maybe the tck test should be updated, because bool is not in the list of invalid types for tointeger() in tck-M21

@codecov
Copy link

codecov bot commented Dec 27, 2022

Codecov Report

Base: 92.16% // Head: 92.11% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (932cc31) compared to base (6e339e9).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2797      +/-   ##
==========================================
- Coverage   92.16%   92.11%   -0.05%     
==========================================
  Files         264      264              
  Lines       25093    25097       +4     
==========================================
- Hits        23126    23118       -8     
- Misses       1967     1979      +12     
Impacted Files Coverage Δ
src/arithmetic/numeric_funcs/numeric_funcs.c 100.00% <100.00%> (ø)
src/procedures/proc_sp_paths.c 95.70% <0.00%> (-1.44%) ⬇️
src/procedures/proc_ss_paths.c 90.69% <0.00%> (-0.88%) ⬇️
src/value.c 90.30% <0.00%> (-0.61%) ⬇️
src/arithmetic/arithmetic_expression.c 93.39% <0.00%> (-0.46%) ⬇️
src/execution_plan/ops/op_aggregate.c 98.76% <0.00%> (ø)

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.

@nafraf nafraf marked this pull request as ready for review December 27, 2022 19:18
@@ -122,6 +122,7 @@ Feature: TypeConversion2 - To Integer
| 42 |
And no side effects

@skip
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

File updated.
Tags @leak and @NegativeTest were removed because are not part of original tck file, but I'm not sure if those should exists in redisgraph project.

@AviAvni AviAvni merged commit f832c7f into RedisGraph:master Dec 29, 2022
@nafraf nafraf deleted the bug#2794 branch December 29, 2022 12:30
raz-mon pushed a commit that referenced this pull request Jan 25, 2023
* AR_TOINTEGER: add case for type T_BOOL

* skip [8] toInteger() failing on invalid arguments

* Update TypeConversion2.feature

Co-authored-by: Avi Avni <avi.avni@gmail.com>
AviAvni added a commit that referenced this pull request Jan 25, 2023
* Fix toInteger() and toIntegerOrNull() don't convert Booleans (#2797)

* AR_TOINTEGER: add case for type T_BOOL

* skip [8] toInteger() failing on invalid arguments

* Update TypeConversion2.feature

Co-authored-by: Avi Avni <avi.avni@gmail.com>

* Fix right(), left() length validation (#2801)

* Fix AR_LEFT, AR_RIGHT length validation

* Test mistmatch type and fix error message

* left(), right(): NULL is a valid type for lenght

* Fix argument validation

Co-authored-by: Avi Avni <avi.avni@gmail.com>

* fix using timeout max as default when default not configured (#2808)

* fix using timeout max as default when default not configured

* fix

* do not cache query params (#2803)

* do not cache query params

* address PR comments

* fix leak

* fix test

* convert params to SIValues

* wip

* delay freeing of parsed params to after the query is parsed

* free parse params when freeing AST

* revert changes to ast_validations.c

* revert arithmetic_expression_construct changes

* emit an error when param value eval fails

Co-authored-by: Avi Avni <avi.avni@gmail.com>

* fix multi-hop traverse with name path (#2743)

* Add varLen property to QGEdge

* Test variable-length edge queries

* Fix spacing

* Fix test, remove direction

* Fix test

* Test zero length edge

* Init varLen in QGEdge_New()

* rename varLen to rangeLen

* Rewrite test11_range_length_edges()

* Use QGEdge_VariableLength() and QGEdge_RangeLength

* Replace QGEdge_RangeLength() by QGEdge_LengthOne()

* rename function

Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>
Co-authored-by: Avi Avni <avi.avni@gmail.com>

* indegree(), outdegree() - accept array of strings (#2807)

* _AR_NodeDegree() support T_ARRAY argument

* Add tests

* Fix argument validation

* Free array in case of error detected.

* Duplicate comments and use siarray.array[i]

* Update AR_IN() to use SI_ArrayContainsValue()

* Validate comparedNull pointer

* Support only the following two signatures:

* Test indegree() wrong argument number

* Test wrong signature: string and list

* clean

* revert

Co-authored-by: Avi Avni <avi.avni@gmail.com>

* Add ipv6 capability (#2842)

* fix heap remove item (#2827)

* fix heap remove item

* remove utf8 deps

* add test

* wip

* wip

* all unit tests pass

* heap remove element should use pointer cmp instead of cmp cb

* spaces to tabs

Co-authored-by: swilly22 <roi@redislabs.com>
Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>

* bump version 2.10.6

* Improve edge deletion performance (#2758)

* improve edge deletion

* update simple timer

* improve edge deletion perf

* fix build

* fix test

* fix test

* clean

* address review

Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>

* update ramp

Co-authored-by: nafraf <nafraf@users.noreply.github.com>
Co-authored-by: Avi Avni <avi.avni@gmail.com>
Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>
Co-authored-by: Guy Korland <gkorland@gmail.com>
Co-authored-by: swilly22 <roi@redislabs.com>
pnxguide pushed a commit to CMU-SPEED/RedisGraph that referenced this pull request Mar 22, 2023
…aph#2797)

* AR_TOINTEGER: add case for type T_BOOL

* skip [8] toInteger() failing on invalid arguments

* Update TypeConversion2.feature

Co-authored-by: Avi Avni <avi.avni@gmail.com>
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.

2 participants