You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/11/17 00:04:20 UTC
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5114
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
[python] added KuduPartialRow::SetNoCopy{Binary,String}()
Added missing cython mappings for KuduPartialRow methods:
* SetNoCopy{Binary,String}
Also, updated the comments for Set{Binary,String} method mappings
to reflect changes on the semantics of those methods.
This is a follow-up for 48766a4ce17d422ced9a6ec78c9a9969ac44d8c9.
Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
---
M python/kudu/libkudu_client.pxd
1 file changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/5114/1
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Patch Set 2:
Ah, i see, so in that case are you thinking we should use the nocopy routine to continue as was done before? If so you'll want to also update the PartialRow stuff, otherwise this code does nothing.
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Patch Set 2:
> didnt finish my sentence above...given the way python handles the
> semantics for PartialRows, im not sure if theres any way to deal
> with the NoCopy setters, also not 100% sure what effect python GC
> would have on using a NoCopy setter.
Well, supposedly it does the same thing as it did for old semantics of SetString/SetBinary -- prior to 48766a4ce17d422ced9a6ec78c9a9969ac44d8c9 they were not copying data. Basically, older Set{String,Binary} behaved like current Set{String,Binary}NoCopy.
If you think it requires additional clarification -- sure, let's do that. I just assumed that since it worked for Set{String,Binary} before, it's going to work for Set{String,Binary}NoCopy now.
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has uploaded a new patch set (#2).
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
[python] added KuduPartialRow::SetNoCopy{Binary,String}()
Added missing cython mappings for KuduPartialRow methods:
* SetNoCopy{Binary,String}
Also, updated the comments for Set{Binary,String} method mappings
to reflect changes on the semantics of those.
This is a follow-up for 48766a4ce17d422ced9a6ec78c9a9969ac44d8c9.
Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
---
M python/kudu/libkudu_client.pxd
1 file changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/5114/2
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Patch Set 2:
I do agree with clarifying the comments either way though
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Patch Set 2:
didnt finish my sentence above...given the way python handles the semantics for PartialRows, im not sure if theres any way to deal with the NoCopy setters, also not 100% sure what effect python GC would have on using a NoCopy setter.
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Patch Set 2:
Was this more for parity's sake or did you need this exposed? The libkudu_client module really just exposes the external signatures to python so if you want this to be exposed in the client it would need to be implemented in the PartialRow class in client.pyx. However, given the way python handles the semantics for PartialRows (schema.new_row({'foo': 'bar'} or alternatively row['foo'] = 'bar'
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Patch Set 2:
> Was this more for parity's sake or did you need this exposed? The
> libkudu_client module really just exposes the external signatures
> to python so if you want this to be exposed in the client it would
> need to be implemented in the PartialRow class in client.pyx.
> However, given the way python handles the semantics for PartialRows
> (schema.new_row({'foo': 'bar'} or alternatively row['foo'] = 'bar'
Yes, that was more about parity and also updating the stale comment about the SetString/SetBinary -- those changed their sematics.
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has abandoned this change.
Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
......................................................................
Abandoned
not needed
--
To view, visit http://gerrit.cloudera.org:8080/5114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>