Skip to content

Fix panic in buildCodecForTypeDescribedByString#289

Open
liron-l wants to merge 1 commit into
linkedin:masterfrom
liron-l:master
Open

Fix panic in buildCodecForTypeDescribedByString#289
liron-l wants to merge 1 commit into
linkedin:masterfrom
liron-l:master

Conversation

@liron-l

@liron-l liron-l commented Oct 23, 2024

Copy link
Copy Markdown

scale attribute is optional in schema:

The following attributes are supported:
• scale, a JSON integer representing the scale (optional). If not specified the scale is 0.
• precision, a JSON integer representing the (maximum) precision of decimals stored
in this type (required)

If it's missing from format, code will panic on

panic: interface conversion: interface {} is nil, not float64

Ensure code robustness and verify both values are cast-able to float64.

scale attribute is optional in schema:
```
The following attributes are supported:
• scale, a JSON integer representing the scale (optional). If not specified the scale is 0.
• precision, a JSON integer representing the (maximum) precision of decimals stored
in this type (required)
```
If it's missing from format, code will panic on
```
panic: interface conversion: interface {} is nil, not float64
```
Ensure code robustness and verify both values are cast-able to float64.
@liron-l

liron-l commented Oct 23, 2024

Copy link
Copy Markdown
Author

@mittal-aashay can you please review?

@liron-l

liron-l commented Dec 17, 2024

Copy link
Copy Markdown
Author

@mittal-aashay who is maintaining this library?

Comment thread codec.go
// using the cached codec in favor of creating a new codec.
if searchType == "bytes.decimal" {
precision, ok1 := schemaMap["precision"].(float64)
scale, ok2 := schemaMap["scale"].(float64)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this default to 0 instead, otherwise we never get a cache hit for something without an explicit scale?

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