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

Stefania commented on CASSANDRA-10624:
--------------------------------------

It LGTM and it is definitely a major improvement compared to where we started 
from. 

I have only really minor nits:

* 
[{{CreateTypeStatement.addToRowBuilder}}|https://github.com/ifesdjeen/cassandra/commit/8074c4df9c93db86cb1e89c84e5921fa3e938433#diff-968b11bb14445fc54d6cc39cd34a8025R102]
 shouldn't be static since we need to pass in a {{CreateTypeStatement}}?
* In CQLSSTableWriterTest.java, I'm not sure if the imports with '*' were 
intentionally replaced with actual individual names or if it was automatically 
done by the IDE
* extra blank line 
[here|https://github.com/ifesdjeen/cassandra/commit/8074c4df9c93db86cb1e89c84e5921fa3e938433#diff-968b11bb14445fc54d6cc39cd34a8025R115]
 and 
[here|https://github.com/ifesdjeen/cassandra/commit/8074c4df9c93db86cb1e89c84e5921fa3e938433#diff-e0bfc4e373099711d483e7cc91616d57R438].

Feel free to ignore the last two points.

BTW, I don't think the {{com.datastax.driver.}} imports should come before the 
o.a.c imports but we do it in several other places as well and the intellij 
project template itself gets it wrong, so I don't think we should fix in this 
patch.

Checked that all the failing utests and dtests either pass locally or fail on 
trunk as well.

So overall +1.

BTW, feel free to remove my name from the author's field, it is really 
[~ifesdjeen] patch! :)

> Support UDT in CQLSSTableWriter
> -------------------------------
>
>                 Key: CASSANDRA-10624
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10624
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: CQL
>            Reporter: Sylvain Lebresne
>            Assignee: Alex Petrov
>             Fix For: 3.x
>
>         Attachments: 0001-Add-support-for-UDTs-to-CQLSStableWriter.patch, 
> 0001-Support-UDTs-in-CQLSStableWriterV2.patch
>
>
> As far as I can tell, there is not way to use a UDT with {{CQLSSTableWriter}} 
> since there is no way to declare it and thus {{CQLSSTableWriter.Builder}} 
> knows of no UDT when parsing the {{CREATE TABLE}} statement passed.
> In terms of API, I think the simplest would be to allow to pass types to the 
> builder in the same way we pass the table definition. So something like:
> {noformat}
> String type = "CREATE TYPE myKs.vertex (x int, y int, z int)";
> String schema = "CREATE TABLE myKs.myTable ("
>               + "  k int PRIMARY KEY,"
>               + "  s set<vertex>"
>               + ")";
> String insert = ...;
> CQLSSTableWriter writer = CQLSSTableWriter.builder()
>                                           .inDirectory("path/to/directory")
>                                           .withType(type)
>                                           .forTable(schema)
>                                           .using(insert).build();
> {noformat}
> I'll note that implementation wise, this might be a bit simpler after the 
> changes of CASSANDRA-10365 (as it makes it easy to passe specific types 
> during the preparation of the create statement).



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

Reply via email to