David Ribeiro Alves has posted comments on this change.

Change subject: [c++ client] - Expose the schema being scanned in KuduScanBatch
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/2020/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

Line 84:   const KuduSchema* projection_schema() const;
> Why expose this in KuduScanBatch/RowPtr and not in KuduScanner? It's a prop
One of the main points of this patch is to ease decoding the results of a scan 
(a KuduScanBatch) without having to infer, keep or pass a schema around, 
exposing the schema that was already contained (but internal to) a 
KuduScanBatch::RowPtr.

Adding such a method to the scanner would not totally achieve the same goal as 
it still would require the user to extract the schema from the scanner and pass 
it around, possibly also keeping that schema as state somewhere or having to 
pass the scanner itself.

It just seems that it is information that is implicit to the scan batch and 
that could/should be exposed by it.

This is not to say that we couldn't the other method too, of course.

Finally RowResult in the java client also exposes the schema, so this would 
still keep the clients consistent.


http://gerrit.cloudera.org:8080/#/c/2020/1/src/kudu/client/schema.h
File src/kudu/client/schema.h:

Line 292:   explicit KuduSchema(const Schema& schema);
> This shouldn't be part of the public API, because kudu::Schema is not a pub
yeah I pondered on that, had a couple of friend methods instead (it was weird 
that I ended up having a couple of them, but maybe that was just my bad and 
could actually be improved) but then realized Schema is, in fact, a public 
class since it's used in KuduPartialRow right?, so no reason not to use it here 
too.

In any case how would you do it?


-- 
To view, visit http://gerrit.cloudera.org:8080/2020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4276036d5ba7faf7959b4324fd013b68a08bec5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to