[ 
https://issues.apache.org/jira/browse/CASSANDRA-11424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15382344#comment-15382344
 ] 

Sylvain Lebresne commented on CASSANDRA-11424:
----------------------------------------------

Thanks for the patch. A few remarks on it though:
* It's still missing allowing the {{DEFAULT}} for named bind markers. And it's 
imo shorter to just handle the option at the statement level (in the parser).
* The {{DEFAULT_NULL}} constant in {{Json.java}} was unused
* I think it's weird to handle bind markers inside {{Json.java}} (in 
{{DelayedColumnValue.bind()}}), but the non-prepared literals in 
{{UpdatedStatement}}. I'd rather keep the logic inside {{Json.java}} and handle 
null/unset with proper values/literals. We also don't really need 
{{defaultNull}} in {{parseJson}}: we can have it set a {{NULL_VALUE}} on an 
explicity user {{null}}, and leave the rest handled out of the method, which 
avoid having to pass the option to {{QueryOptions}} (not a big deal, but 
slightly simplify things).
* The code source uses spaces not tabs for indentation.

I've pushed a modification below of what I have in mind. I've also included 
short modification of the doc and started CI on this. Let me know if you're 
happy with this.
| [11424-trunk|https://github.com/pcmanus/cassandra/commits/11424-trunk] | 
[utests|http://cassci.datastax.com/job/pcmanus-11424-trunk-testall] | 
[dtests|http://cassci.datastax.com/job/pcmanus-11424-trunk-dtest] |

As an aside, it's worth noting that this "default" only apply to top-level 
columns, and won't apply to UDT nested inside the JSON, even if those are non 
frozen. I've wondered if it shouldn't apply to those nested UDT too, but it's 
actually a bit annoying to do cause setting UDT value, even non-frozen ones, 
delete previous values (as it should), and making a difference here would 
require a bunch of ugly code. Further, I kind of feel that making that kind of 
"special" case for UDT could be weird: we'd still overide collections fully for 
instance, but not UDT? Anyway, I "think" only applying this default for 
top-level column is correct, but wanted to noted my reflexion for the records.


> Option to leave omitted columns in INSERT JSON unset
> ----------------------------------------------------
>
>                 Key: CASSANDRA-11424
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11424
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Ralf Steppacher
>            Assignee: Oded Peer
>              Labels: client-impacting, cql
>             Fix For: 3.8
>
>         Attachments: 11424-trunk-V1.txt, 11424-trunk-V2.txt, 
> 11424-trunk-V3.txt
>
>
> CASSANDRA-7304 introduced the ability to distinguish between {{NULL}} and 
> {{UNSET}} prepared statement parameters.
> When inserting JSON objects it is not possible to profit from this as a 
> prepared statement only has one parameter that is bound to the JSON object as 
> a whole. There is no way to control {{NULL}} vs {{UNSET}} behavior for 
> columns omitted from the JSON object.
> Please extend on CASSANDRA-7304 to include JSON support.
> {color:grey}
> (My personal requirement is to be able to insert JSON objects with optional 
> fields without incurring the overhead of creating a tombstone of every column 
> not covered by the JSON object upon initial(!) insert.)
> {color}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to