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.

Sign up for GitHub

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

Jump to bottom

JSONB_POPULATE_RECORD cannot convert jsonb strings to number type #19918

Closed
fuyufjh opened this issue Dec 24, 2024 · 1 comment · Fixed by #19937
Closed

JSONB_POPULATE_RECORD cannot convert jsonb strings to number type #19918

fuyufjh opened this issue Dec 24, 2024 · 1 comment · Fixed by #19937
Labels
type/bug Something isn't working
Milestone
release-2.2

Comments

@fuyufjh
Copy link
Member

fuyufjh commented Dec 24, 2024 •
edited
Loading

Describe the bug

JSONB_POPULATE_RECORD & JSONB_POPULATE_RECORDSET cannot convert jsonb strings to any SQL number type

Besides, parsing orders of magnitudes slower when schema set at source level versus reading in the raw jsonb and using a function like JSONB_POPULATE_RECORD in an MV (slack archive)

Error message/log

No response

To Reproduce

No response

Expected behavior

Better to try_parse the string instead of reporting error and leaving a null.

PostgresQL:

test=# CREATE TYPE test_table_type AS (id BIGINT);
CREATE TYPE
test=# select JSONB_POPULATE_RECORD(NULL::test_table_type, '{"id": "12345"}'::jsonb);
 jsonb_populate_record
-----------------------
 (12345)
(1 row)

How did you deploy RisingWave?

No response

The version of RisingWave

No response

Additional context

No response
@fuyufjh fuyufjh added the type/bug Something isn't working label Dec 24, 2024
@github-actions github-actions bot added this to the release-2.2 milestone Dec 24, 2024
@fuyufjh
Copy link
Member Author

fuyufjh commented Dec 26, 2024

Analysis:

populate_struct calls JsonbRef::to_datum(ty) to convert JSON value to datum.

risingwave/src/common/src/types/jsonb.rs

Line 523 in 119e17e

_ => Self(v).to_datum(ty)?,

For numbers, it only calls jsonbb::ValueRef::as_number(), thus string values will cause an error.

risingwave/src/common/src/types/jsonb.rs

Lines 432 to 436 in 119e17e

DataType::Int16 => ScalarImpl::Int16(self.as_number()?.try_into()?),
DataType::Int32 => ScalarImpl::Int32(self.as_number()?.try_into()?),
DataType::Int64 => ScalarImpl::Int64(self.as_number()?.try_into()?),
DataType::Float32 => ScalarImpl::Float32(self.as_number()?.try_into()?),
DataType::Float64 => ScalarImpl::Float64(self.as_number()?),

Need to modify here to parse string values as well.

@zwang28 zwang28 mentioned this issue Dec 26, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/bug Something isn't working
Projects
None yet
Milestone
release-2.2
Development

Successfully merging a pull request may close this issue.

fix: support string to number type in JSONB_POPULATE_RECORD risingwavelabs/risingwave
1 participant
@fuyufjh