-
Notifications
You must be signed in to change notification settings - Fork 567
Don't fail when inserting UDTs with prepared queries with some missing fields #224 #1151
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
Conversation
|
Thanks for the PR @Mokto! Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks! |
|
I just did ;) |
|
Hi, Thanks. |
fruch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@Mokto Apologies for the delay in getting back to this; we've got a lot of balls in the air at the moment. A few notes, perhaps mostly to myself. First off this works fine via CQL: Second this change really only affects the prepared statement case. When an "incomplete" (for lack of a better term) instance representing a UDT type is fed to a simple statement the input type is converted to something like the UDT literal above so you get basically the same behaviour. In the prepared statement case we actually know the type that's expected so we can attempt to serialize based on cassandra.cqltypes.UserType, which lands us in the code cited above. Finally, an explicit repro case: from cassandra.cluster import Cluster
cluster = Cluster()
session = cluster.connect()
session.execute("drop keyspace if exists mykeyspace")
session.execute("create KEYSPACE mykeyspace WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 1}")
session.set_keyspace('mykeyspace')
session.execute("CREATE TYPE address (street text, zipcode int, raw_address text)")
session.execute("CREATE TABLE users (id int PRIMARY KEY, location frozen<address>)")
# create a class to map to the "address" UDT
class Address(object):
def __init__(self, street, zipcode):
self.street = street
self.zipcode = zipcode
#cluster.register_user_type('mykeyspace', 'address', Address)
insert_statement = session.prepare("INSERT INTO users (id, location) VALUES (?, ?)")
session.execute(insert_statement, [0, Address("123 Main St.", 78723)])Without this change (and using the repro code above) you get: With the fix you get no error and a row matching the CQL example above added. |
|
Thanks! |
…sync_with_upstream_3.29.1 version 3.28.0 * tag '3.28.0' of https://github.com/datastax/python-driver: Release 3.28.0: changelog & version PYTHON-1352 Add vector type, codec + support for parsing CQL type (apache#1161) Update docs.yaml to point to most recent 3.27.0 docs changes CONN-38 Notes for 3.27.0 on PYTHON-1350 (apache#1166) PYTHON-1356 Create session-specific protocol handlers to contain session-specific CLE policies (apache#1165) PYTHON-1350 Store IV along with encrypted text when using column-level encryption (apache#1160) PYTHON-1351 Convert cryptography to an optional dependency (apache#1164) Jenkinsfile cleanup (apache#1163) PYTHON-1343 Use Cython for smoke builds (apache#1162) Don't fail when inserting UDTs with prepared queries with some missing fields (apache#1151) Revert "remove unnecessary import __future__ (apache#1156)" docs: convert print statement to function in docs (apache#1157) remove unnecessary import __future__ (apache#1156) Update docs.yaml to include recent fixes to CLE docs Fix for rendering of code blocks in CLE documentation (apache#1159) DOC-3278 Update comment for retry policy (apache#1158) DOC-2813 (apache#1145) Remove different build matrix selection for develop branches (apache#1138)
Let's say you have an object:
And let's say the type actually contains another field, let's call i raw_address
Then inserting data through a prepared statement will actually fail : the driver will complain raw_address is missing.
This change addresses that, as any field should be optional.