cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefania (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-10624) Support UDT in CQLSSTableWriter
Date Wed, 06 Apr 2016 03:06:25 GMT

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

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

This patch is really good and I am really impressed with how quickly you created it.

I have only one major suggestion, which is to get rid of the codec map and the list of user
types in the builder, and instead use Types in the builder and then add them to the keyspace
metadata. This has also the advantage that duplicate type names are detected when calling
withType. It's easier to show, so I created a commit with some suggestions [here|https://github.com/stef1927/cassandra/commit/afe170ad07045e75ad0f7c96e4dcedc03ca3230b].

*Nits*
* In CQLSStableWriter.getTableMetadata() at line 562 the comment is no longer relevant.
* In this same method, it would be nice to have an IllegalArgumentException check, just like
you did for withType.
* In some locations the curly brackets are not positioned on a new line, I spotted them in
CQLSStableWriter at lines 295, 609 and in CQLSStableWriterTest at line 359.

I think only the second point was not done in the commit with suggestions above, but double
check on the brackets.

*Continuous integration*
I've launched some jobs against my GH branch, they are still running:

http://cassci.datastax.com/job/stef1927-10624-testall
http://cassci.datastax.com/job/stef1927-10624-dtest

You can ping "exlt" or "ptnapoleon" in IRC to set-up your GH account to run CI on cassci.datastax.com.

> 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
>
>
> 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)

Mime
View raw message