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

CLI: fix binary syntax has a bug to add one backtick #352

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

chidea
Copy link

@chidea chidea commented Jun 5, 2024

How to reproduce bug

create space test
create model test.test(id:uint8, v:binary)
insert into test.test(0, ``)
select v from test.test where id=0

and the result is ([96]) where it must be ([]).

Reason

The closing backtick(ascii:96) was added into query parameter because of wrong range to take.


✔️ By submitting this pull request, I agree to the CLA at: https://cla.skytable.io/skytable/skytable

@chidea chidea requested a review from ohsayan as a code owner June 5, 2024 03:09
@chidea chidea changed the title fix : cli binary syntax has a bug to add one backtick CLI: fix binary syntax has a bug to add one backtick Jun 5, 2024
@BitHeaven-Official
Copy link

BitHeaven-Official commented Jun 5, 2024

You need use " or ' for Values.
96 is dec val of bin ascii ` (` (ascii) -> 01100000 (bin) -> 96 (dec))

@chidea
Copy link
Author

chidea commented Jun 7, 2024

@BitHeaven-Official " or ' is for string type values and ` is for binary type values. While string types are read by Parameterizer::read_string function, binary types are read by Parameterizer::read_binary function.
cli/src/query.rs:145

Copy link
Member

@ohsayan ohsayan left a comment

Choose a reason for hiding this comment

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

Good catch! Do you think you'll be able to add some tests? If not, no issues...I'll add them later :)

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.

None yet

3 participants