JSONExtract String or Raw

[amosbird]

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

• Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Allow extract non-string element as string using JSONExtract. This is for #25414

Detailed description / Documentation draft:

.

[Avogar]

The changes also affect JSONExtractKeysAndValues function:

SELECT JSONExtractKeysAndValues('{"a": "string", "b": [1, 2]}', 'String') = [('a','string'),('b','[1,2]')]

Do we really want such behaviour?

[amosbird]

Do we really want such behaviour?

Looks ok to me)

[Avogar]

Ok, let's fix tests and merge it

[filimonov]

The main problem in that implementation is ambiguity of

{"a": "{}", "b": {} }

And lack of possibility to extract string field with surrounding quotes (to have always valid sub-json, which can be parsed)

It is rather minor issue, but it potentially is not 100% sane, and once it will be merged we will not be able to fix it anymore in any other way. String will always extract raw-json.

[alexey-milovidov]

@filimonov This is what JSONExtractRaw for.

[filimonov]

@filimonov This is what JSONExtractRaw for.

Check #25414

[Avogar]

@filimonov maybe you have an idea how to solve this ambiguity? One of the possible solution is to add new data type RawString, but this is overkill I think.

[amosbird]

@filimonov maybe you have an idea how to solve this ambiguity? One of the possible solution is to add new data type RawString, but this is overkill I think.

It's also possible to give a different name like JSONExtractWithRawString. However, I don't understand the impact of this ambiguity. What would break?

[filimonov]

@filimonov maybe you have an idea how to solve this ambiguity?

Nothing much smarter than:

to add new data type RawString
but this is overkill I think.

Sounds like overkill, agree.

It's also possible to give a different name like JSONExtractWithRawString.

Theoretically it's bit safer (no regressions like in example below). But i also don't feel 100% happy about that (more functions / switches again)

However, I don't understand the impact of this ambiguity. What would break?

For ex. things like

with '{a:{b:1}}' as json
Select if(JSONExtract(json, 'a'), JSONExtract(json, 'a'), JSONExtract(json, 'a.b'))

[alexey-milovidov]

@Mergifyio update

[mergify]

Command update: success

Branch has been successfully updated

[filimonov]

That needs a clear documentation. Also a note in changelog.

[wangqinghuan]

This change does not affect JSONExtractString(). Could we keep same behavior in all JSONExtract String function？ @amosbird

[amosbird]

@wangqinghuan Yes. It's reasonable. #30426