Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: add missing handler for deserializing json value - #1587

Merged
tswast merged 7 commits into
googleapis:mainfrom
tomwojcik:feature/#1500-jsonfield-deserialize
Dec 13, 2023
Merged

fix: add missing handler for deserializing json value#1587
tswast merged 7 commits into
googleapis:mainfrom
tomwojcik:feature/#1500-jsonfield-deserialize

Conversation

@tomwojcik

Copy link
Copy Markdown
Contributor

Fixes#1500

@tomwojcik
tomwojcik requested review from a team and tswastJune 19, 2023 10:59
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@product-auto-labelproduct-auto-labelBot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Jun 19, 2023
@tomwojcik

tomwojcik commented Jul 2, 2023

Copy link
Copy Markdown
ContributorAuthor

@tswast bump, please review

@partheaparthea added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Jul 6, 2023
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 6, 2023
@yoshi-kokoroyoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 6, 2023
@tswasttswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 3, 2023
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 3, 2023

@tswasttswast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

One concern I have is the other direction. If someone supplies a parsed JSON object to an API like insert_rows, do we correctly serialize that?

@LinchinLinchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 1, 2023
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 1, 2023
@tswasttswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 13, 2023
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 13, 2023
@tswast

Copy link
Copy Markdown
Contributor

Looks like everything is working well except for the type annotations:

nox > Running session mypy
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/mypy
nox > python -m pip install -e '.[all]'
nox > python -m pip install mypy==1.6.1
nox > python -m pip install types-protobuf types-python-dateutil types-requests types-setuptools
nox > mypy -p google --show-traceback
google/cloud/bigquery/query.py:472: error: Cannot call function of unknown type [operator]
google/cloud/bigquery/query.py:629: error: Cannot call function of unknown type [operator]
google/cloud/bigquery/query.py:778: error: Cannot call function of unknown type [operator]
Found 3 errors in 1 file (checked 54 source files)
nox > Command mypy -p google --show-traceback failed with exit code 1
nox > Session mypy failed.

@tswasttswast self-assigned this Dec 13, 2023
@tswasttswast added kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Dec 13, 2023
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Dec 13, 2023
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 13, 2023
@tswast
tswast merged commit 09017a9 into googleapis:mainDec 13, 2023
@release-pleaserelease-pleaseBot mentioned this pull request Dec 13, 2023
@tomwojcik
tomwojcik deleted the feature/#1500-jsonfield-deserialize branch December 13, 2023 22:48
@MartijnvanElferen

Copy link
Copy Markdown

This change is causing the following error for insert_rows:

TypeError: _json_from_json() missing 1 required positional argument: 'field'

Is there a recommended fix for this?

@tswast

Copy link
Copy Markdown
Contributor

Thanks @MartijnvanElferen for the report. I've filed #1756 to investigate further.

@tswast

Copy link
Copy Markdown
Contributor

As a workaround insert_rows_json should avoid this type conversion code, but you're responsible for putting things in a format the BigQuery REST API understands.

@tswast

Copy link
Copy Markdown
Contributor

@MartijnvanElferen Fix pending: #1757

Note: this PR will change the behavior of JSON data in insert_rows to call json.dumps, which is consistent with the new behavior of calling json.loads when reading row values. Previously, JSON support was happening via a generic fallback that didn't do any conversion of values passed in.

@MartijnvanElferen

Copy link
Copy Markdown

Thanks, @tswast. I'll refactor a couple of things to avoid the type conversion. Looking forward to the fix at the same time!

@kcooney

kcooney commented May 31, 2024

Copy link
Copy Markdown

@tswast This change should probably be listed as a potentially braking change in the release notes, since before this change reading the entire column would return a value of type str:

CREATETABLEmydataset.table(
id INT64,
proto_as_json JSON
);
fromgoogle.cloudimportbigqueryfromgoogle.protobufimportjson_formatfromcom.example.my_proto_pb2importMyProtoclient=bigquery.Client()
proto=MyProto(first_name="Jane", last_name="Doe") proto_as_json=json_format.MessageToJson(
proto, use_integers_for_enums=True, indent=None)
rows_to_insert= [
{"id": 1, "proto_as_json": f'JSON """{proto_as_json}"""'},
]
errors=client.insert_rows("mydataset.table", rows_to_insert)
assertnoterrorsrow=client.query("SELECT proto_as_json FROM mydataset.table").result().__next__()
json_str=row["proto_as_json"]
# Prior to this change, json_str was of type str, so we could do this:read_proto=MyProto()
json_format.Parse(json_str, read_proto)
assertproto==read_proto

Luckily, in our project we created a common helper method for the json_format.Parse() call, so we can update it to use json_format.ParseDict()

@tswast

Copy link
Copy Markdown
Contributor

Yeah, it's a gray area with this since we never said we support the JSON type until this change.

We still don't support JSON in some code paths like to_dataframe()

@kcooney

kcooney commented May 31, 2024

Copy link
Copy Markdown

@tswast We saw the breakage when we tried to update a number of python libraries to newer versions. The release notes were very unhelpful in determining which library and version could have caused the problem (because the PR and bug listed in the release notes suggested that it only affected selecting subfields). It wasn't until we found a work around that we were able to track the issue to v3.15.0 and this change. Even if there were no prior promises of support of the JSON type, a short blurb in the release notes would be very much appreciated.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigqueryIssues related to the googleapis/python-bigquery API.size: sPull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deserializing JSON subfields within structs fails

7 participants

@tomwojcik@tswast@MartijnvanElferen@kcooney@parthea@Linchin@yoshi-kokoro