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

linter: fixed unboxing from null for arrays #1215

Merged
merged 6 commits into from
Jul 25, 2024

Conversation

Hidanio
Copy link
Contributor

@Hidanio Hidanio commented Mar 13, 2024

No description provided.

@Hidanio Hidanio added the bug Something isn't working label Mar 13, 2024
@Hidanio Hidanio self-assigned this Mar 13, 2024
@Hidanio Hidanio linked an issue Mar 13, 2024 that may be closed by this pull request
@YuriyNasretdinov
Copy link
Contributor

Seems quite non-obvious to me why does UnwrapArray actually panic in this case...

@Hidanio
Copy link
Contributor Author

Hidanio commented Mar 14, 2024

Seems quite non-obvious to me why does UnwrapArray actually panic in this case...

@YuriyNasretdinov The problem in unwrap1 function:

const stringLenBytes = 4

func unwrap1(s string) (one string) {
	return s[stringLenBytes+1:] // do not care about length, there is only 1 param
}

if we have "null" string, then we will have slice from 5, but we have only 3 indexes. we can modify unwrap1 function to return empty string if len(s) > stringLenBytes+1, but we are talking about "types" and "null" is a special type that`s why i think we should add condition in logic of LazyArrayElemType. What do you think?

@YuriyNasretdinov
Copy link
Contributor

Thanks for the explanation. I think it makes sense now. BTW I am not a maintainer of the project (but I was at some point), so I'm just being curious :).

src/types/map.go Outdated Show resolved Hide resolved
src/types/map.go Outdated Show resolved Hide resolved
src/tests/regression/issue1214_test.go Outdated Show resolved Hide resolved
@VKCOM VKCOM deleted a comment from Hidanio Jul 24, 2024
@Hidanio Hidanio force-pushed the hidanio/fix_panic_null_unboxing branch from 6e763bf to d5db9fe Compare July 24, 2024 16:26
src/tests/exprtype/exprtype_test.go Outdated Show resolved Hide resolved
src/types/map.go Outdated Show resolved Hide resolved
src/types/map.go Outdated Show resolved Hide resolved
@Hidanio Hidanio force-pushed the hidanio/fix_panic_null_unboxing branch 2 times, most recently from 8df655a to 6d9476b Compare July 24, 2024 18:23
src/solver/solver.go Show resolved Hide resolved
src/tests/exprtype/exprtype_test.go Outdated Show resolved Hide resolved
@Hidanio Hidanio force-pushed the hidanio/fix_panic_null_unboxing branch from f5b8a2e to 99484be Compare July 25, 2024 11:32
@Hidanio Hidanio merged commit fa7b184 into master Jul 25, 2024
2 checks passed
@Hidanio Hidanio deleted the hidanio/fix_panic_null_unboxing branch July 25, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when trying to unboxing variable that can be null
3 participants