{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/9169","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/9169/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/9169/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/9169/events","html_url":"https://github.com/cockroachdb/cockroach/pull/9169","id":175448722,"number":9169,"title":"add log in client.Batch","user":{"login":"a6802739","id":3368692,"avatar_url":"https://avatars.githubusercontent.com/u/3368692?v=3","gravatar_id":"","url":"https://api.github.com/users/a6802739","html_url":"https://github.com/a6802739","followers_url":"https://api.github.com/users/a6802739/followers","following_url":"https://api.github.com/users/a6802739/following{/other_user}","gists_url":"https://api.github.com/users/a6802739/gists{/gist_id}","starred_url":"https://api.github.com/users/a6802739/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/a6802739/subscriptions","organizations_url":"https://api.github.com/users/a6802739/orgs","repos_url":"https://api.github.com/users/a6802739/repos","events_url":"https://api.github.com/users/a6802739/events{/privacy}","received_events_url":"https://api.github.com/users/a6802739/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":27,"created_at":"2016-09-07T08:57:43Z","updated_at":"2016-09-09T19:32:35Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/9169","html_url":"https://github.com/cockroachdb/cockroach/pull/9169","diff_url":"https://github.com/cockroachdb/cockroach/pull/9169.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/9169.patch"},"body":"@paperstreet, please look at #8324 and tell me if I done it in a right direction.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/cockroachdb/cockroach/9169)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/246015907","html_url":"https://github.com/cockroachdb/cockroach/pull/9169#issuecomment-246015907","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/9169","id":246015907,"user":{"login":"RaduBerinde","id":16544120,"avatar_url":"https://avatars.githubusercontent.com/u/16544120?v=3","gravatar_id":"","url":"https://api.github.com/users/RaduBerinde","html_url":"https://github.com/RaduBerinde","followers_url":"https://api.github.com/users/RaduBerinde/followers","following_url":"https://api.github.com/users/RaduBerinde/following{/other_user}","gists_url":"https://api.github.com/users/RaduBerinde/gists{/gist_id}","starred_url":"https://api.github.com/users/RaduBerinde/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RaduBerinde/subscriptions","organizations_url":"https://api.github.com/users/RaduBerinde/orgs","repos_url":"https://api.github.com/users/RaduBerinde/repos","events_url":"https://api.github.com/users/RaduBerinde/events{/privacy}","received_events_url":"https://api.github.com/users/RaduBerinde/received_events","type":"User","site_admin":false},"created_at":"2016-09-09T19:32:34Z","updated_at":"2016-09-09T19:32:34Z","body":"\n\n\n\nReview status: all files reviewed at latest revision, 7 unresolved discussions, all commit checks successful.\n\n---\n\n*[sql/rowwriter.go, line 197 \\[r6\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/9169#-KRFMDIx0D7kKcnpQLLT:-KRFNE5uwpSjjnzGCKSQ:bd9jjqv) ([raw file](https://github.com/cockroachdb/cockroach/blob/53e979083cfd9a7af0009fb5dcbd5e23411c7a42/sql/rowwriter.go#L197)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> Unfortunately, I think that means we need to introduce a family of `Depth` functions on `Batch` instead of the current approach.\n> \n> So for instance the current `(*Batch).Put` becomes `(*Batch).PutDepth` and a new function `(*Batch).Put(...)` would call `(*Batch).PutDepth(..., 1)`\n> \n> @raduberinde WDYT?\n</details>\nI see two other options:\n - just do `if ignoreConflicts` in the three places where we're actually using this. I have a feeling that might have been the original code anyway if we didn't also have the logging code (@paperstreet - what do you think?)\n - add a ``Batch.PutIfNil` (or `CPutIfNil?`) method which is equivalent to `CPut(k, v, nil)`. Then we can do:\n```\nvar putFn func(*Batch, *Key, *Value)\nif ignoreConflicts {\n  putFn = (*Batch).Put\n} else {\n  putFn = (*Batch).PutIfNil\n}\n```\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cockroachdb/cockroach/9169)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"RaduBerinde"}}