Author: jbellis Date: Sun Apr 17 05:10:17 2011 New Revision: 1094102 URL: http://svn.apache.org/viewvc?rev=1094102&view=rev Log: preserve column order in CQL result sets patch by jbellis; reviewed by eevans for CASSANDRA-2493
Modified: cassandra/branches/cassandra-0.8/CHANGES.txt cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/CassandraResultSet.java cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/ColumnDecoder.java cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/TypedColumn.java cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/JdbcDriverTest.java cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/jdbc/PreparedStatementTest.java cassandra/branches/cassandra-0.8/interface/cassandra.thrift cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Column.java cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Constants.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cli/CliClient.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/hadoop/ColumnFamilyRecordWriter.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/CassandraServer.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java cassandra/branches/cassandra-0.8/test/system/test_cql.py cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/client/TestRingCache.java cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/service/EmbeddedCassandraServiceTest.java Modified: cassandra/branches/cassandra-0.8/CHANGES.txt URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/CHANGES.txt?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/CHANGES.txt (original) +++ cassandra/branches/cassandra-0.8/CHANGES.txt Sun Apr 17 05:10:17 2011 @@ -4,7 +4,7 @@ (CASSANDRA-1072, 1937, 1944, 1936, 2101, 2093, 2288, 2105, 2384, 2236, 2342, 2454) * CQL (CASSANDRA-1703, 1704, 1705, 1706, 1707, 1708, 1710, 1711, 1940, - 2124, 2302, 2277) + 2124, 2302, 2277, 2493) * avoid double RowMutation serialization on write path (CASSANDRA-1800) * make NetworkTopologyStrategy the default (CASSANDRA-1960) * configurable internode encryption (CASSANDRA-1567, 2152) Modified: cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/CassandraResultSet.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/CassandraResultSet.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/CassandraResultSet.java (original) +++ cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/CassandraResultSet.java Sun Apr 17 05:10:17 2011 @@ -75,11 +75,13 @@ class CassandraResultSet implements Resu private List<TypedColumn> values = new ArrayList<TypedColumn>(); /** The value map. */ + // TODO should map <String, TypedColumn> so we can throw appropriate exception if user asks for non-existant column name private Map<String, Object> valueMap = new WeakHashMap<String, Object>(); private final RsMetaData meta; private final AbstractType nameType; + private boolean wasNull; /** * Instantiates a new cassandra result set. @@ -356,7 +358,11 @@ class CassandraResultSet implements Resu */ public byte[] getBytes(int index) throws SQLException { - return values.get(index-1) != null ? ((ByteBuffer)values.get(index-1).getValue()).array() : null; + TypedColumn column = values.get(index - 1); + assert column != null; + Object value = column.getValue(); + wasNull = value == null; + return value == null ? null : ((ByteBuffer) value).array(); } /** @@ -367,7 +373,9 @@ class CassandraResultSet implements Resu public byte[] getBytes(String name) throws SQLException { String nameAsString = decoder.colNameAsString(keyspace, columnFamily, name); - return valueMap.get(nameAsString) != null ? ((ByteBuffer)valueMap.get(nameAsString)).array() : null; + Object value = valueMap.get(nameAsString); + wasNull = value == null; + return value == null ? null : ((ByteBuffer) value).array(); } /** @@ -544,7 +552,11 @@ class CassandraResultSet implements Resu */ public int getInt(int index) throws SQLException { - return values.get(index-1) != null ? ((BigInteger)values.get(index-1).getValue()).intValue() : null; + TypedColumn column = values.get(index - 1); + assert column != null; + Object value = column.getValue(); + wasNull = value == null; + return value == null ? 0 : ((BigInteger) value).intValue(); } /** @@ -555,7 +567,9 @@ class CassandraResultSet implements Resu public int getInt(String name) throws SQLException { String nameAsString = decoder.colNameAsString(keyspace, columnFamily, name); - return valueMap.get(nameAsString) != null ? ((BigInteger)valueMap.get(nameAsString)).intValue() : null; + Object value = valueMap.get(nameAsString); + wasNull = value == null; + return value == null ? 0 : ((BigInteger) value).intValue(); } /** @@ -565,7 +579,12 @@ class CassandraResultSet implements Resu */ public long getLong(int index) throws SQLException { - return values.get(index-1) != null ? (Long)values.get(index-1).getValue() : null; + assert values != null; + TypedColumn column = values.get(index - 1); + assert column != null; + Object value = column.getValue(); + wasNull = value == null; + return value == null ? 0 : (Long) value; } /** @@ -576,7 +595,9 @@ class CassandraResultSet implements Resu public long getLong(String name) throws SQLException { String nameAsString = decoder.colNameAsString(keyspace, columnFamily, name); - return valueMap.get(nameAsString) != null ? (Long)valueMap.get(nameAsString) : null; + Object value = valueMap.get(nameAsString); + wasNull = value == null; + return value == null ? 0 : (Long) value; } /** @@ -655,7 +676,11 @@ class CassandraResultSet implements Resu */ public Object getObject(int index) throws SQLException { - return values.get(index-1) == null ? null : values.get(index-1).getValue(); + TypedColumn column = values.get(index - 1); + assert column != null; + Object value = column.getValue(); + wasNull = value == null; + return value; } /** @@ -666,7 +691,9 @@ class CassandraResultSet implements Resu public Object getObject(String name) throws SQLException { String nameAsString = decoder.colNameAsString(keyspace, columnFamily, name); - return valueMap.get(nameAsString); + Object value = valueMap.get(nameAsString); + wasNull = value == null; + return value; } /** @@ -796,7 +823,11 @@ class CassandraResultSet implements Resu */ public String getString(int index) throws SQLException { - return values.get(index-1) != null ? ColumnDecoder.colValueAsString(values.get(index-1).getValue()) : null; + TypedColumn column = values.get(index - 1); + assert column != null; + Object value = column.getValue(); + wasNull = value == null; + return value == null ? null : ColumnDecoder.colValueAsString(value); } /** @@ -807,7 +838,9 @@ class CassandraResultSet implements Resu public String getString(String name) throws SQLException { String nameAsString = this.decoder.colNameAsString(this.keyspace, this.columnFamily, name); - return valueMap.get(nameAsString) != null ? ColumnDecoder.colValueAsString(valueMap.get(nameAsString)) : null; + Object value = valueMap.get(nameAsString); + wasNull = value == null; + return value == null ? null : ColumnDecoder.colValueAsString(value); } /** @@ -1999,7 +2032,7 @@ class CassandraResultSet implements Resu */ public boolean wasNull() throws SQLException { - throw new UnsupportedOperationException("method not supported"); + return wasNull; } /** Modified: cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/ColumnDecoder.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/ColumnDecoder.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/ColumnDecoder.java (original) +++ cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/ColumnDecoder.java Sun Apr 17 05:10:17 2011 @@ -181,7 +181,7 @@ class ColumnDecoder else if (value instanceof byte[]) return ByteBufferUtil.bytesToHex(ByteBuffer.wrap((byte[])value)); else - return value.toString(); + return value == null ? null : value.toString(); } /** constructs a typed column */ Modified: cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/TypedColumn.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/TypedColumn.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/TypedColumn.java (original) +++ cassandra/branches/cassandra-0.8/drivers/java/src/org/apache/cassandra/cql/jdbc/TypedColumn.java Sun Apr 17 05:10:17 2011 @@ -39,11 +39,11 @@ class TypedColumn<N, V> public TypedColumn(AbstractType<N> comparator, byte[] name, AbstractType<V> validator, byte[] value) { ByteBuffer bbName = ByteBuffer.wrap(name); - ByteBuffer bbValue = ByteBuffer.wrap(value); + ByteBuffer bbValue = value == null ? null : ByteBuffer.wrap(value); this.name = comparator.compose(bbName); - this.value = validator.compose(bbValue); + this.value = value == null ? null : validator.compose(bbValue); nameString = comparator.getString(bbName); - valueString = validator.getString(bbValue); + valueString = value == null ? null : validator.getString(bbValue); this.validator = validator; } Modified: cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/JdbcDriverTest.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/JdbcDriverTest.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/JdbcDriverTest.java (original) +++ cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/JdbcDriverTest.java Sun Apr 17 05:10:17 2011 @@ -398,8 +398,11 @@ public class JdbcDriverTest extends Embe { executeNoResults(con, statements[3*i]); ResultSet rs = executePreparedStatementWithResults(con, statements[3*i+1]); - assert !rs.next() : statements[3*i+1]; + rs.next(); + rs.getObject(1); + assert rs.wasNull(); rs.close(); + rs = executePreparedStatementWithResults(con, statements[3*i+2]); assert rs.next() : statements[3*i+2]; } Modified: cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/jdbc/PreparedStatementTest.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/jdbc/PreparedStatementTest.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/jdbc/PreparedStatementTest.java (original) +++ cassandra/branches/cassandra-0.8/drivers/java/test/org/apache/cassandra/cql/jdbc/PreparedStatementTest.java Sun Apr 17 05:10:17 2011 @@ -80,7 +80,8 @@ public class PreparedStatementTest exten stmt.setBytes(2, FBUtilities.toByteArray(i+100)); stmt.setBytes(3, key); ResultSet rs = stmt.executeQuery(); - assert !rs.next(); + rs.next(); + assert rs.getString(1) == null; assert rs.getString(2) == null; rs.close(); } } @@ -139,7 +140,8 @@ public class PreparedStatementTest exten stmt.setString(2, "2\u6543\u3435\u6554"); stmt.setBytes(3, key); ResultSet rs = stmt.executeQuery(); - assert !rs.next(); + rs.next(); + assert rs.getString(1) == null; assert rs.getString(2) == null; rs.close(); } } @@ -198,7 +200,8 @@ public class PreparedStatementTest exten stmt.setString(2, "2"); stmt.setBytes(3, key); ResultSet rs = stmt.executeQuery(); - assert !rs.next(); + rs.next(); + assert rs.getString(1) == null; assert rs.getString(2) == null; rs.close(); } } @@ -257,7 +260,9 @@ public class PreparedStatementTest exten stmt.setLong(2, 2); stmt.setBytes(3, key); ResultSet rs = stmt.executeQuery(); - assert !rs.next(); + rs.next(); + rs.getLong(1); + assert rs.wasNull(); rs.close(); } } @@ -316,7 +321,9 @@ public class PreparedStatementTest exten stmt.setInt(2, 2); stmt.setBytes(3, key); ResultSet rs = stmt.executeQuery(); - assert !rs.next(); + rs.next(); + rs.getInt(1); + assert rs.wasNull(); rs.close(); } } Modified: cassandra/branches/cassandra-0.8/interface/cassandra.thrift URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/interface/cassandra.thrift?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/interface/cassandra.thrift (original) +++ cassandra/branches/cassandra-0.8/interface/cassandra.thrift Sun Apr 17 05:10:17 2011 @@ -46,7 +46,7 @@ namespace rb CassandraThrift # for every edit that doesn't result in a change to major/minor. # # See the Semantic Versioning Specification (SemVer) http://semver.org. -const string VERSION = "20.1.0" +const string VERSION = "20.2.0" # @@ -61,8 +61,8 @@ const string VERSION = "20.1.0" */ struct Column { 1: required binary name, - 2: required binary value, - 3: required i64 timestamp, + 2: optional binary value, + 3: optional i64 timestamp, 4: optional i32 ttl, } Modified: cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Column.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Column.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Column.java (original) +++ cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Column.java Sun Apr 17 05:10:17 2011 @@ -139,9 +139,9 @@ public class Column implements org.apach Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> tmpMap = new EnumMap<_Fields, org.apache.thrift.meta_data.FieldMetaData>(_Fields.class); tmpMap.put(_Fields.NAME, new org.apache.thrift.meta_data.FieldMetaData("name", org.apache.thrift.TFieldRequirementType.REQUIRED, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING , true))); - tmpMap.put(_Fields.VALUE, new org.apache.thrift.meta_data.FieldMetaData("value", org.apache.thrift.TFieldRequirementType.REQUIRED, + tmpMap.put(_Fields.VALUE, new org.apache.thrift.meta_data.FieldMetaData("value", org.apache.thrift.TFieldRequirementType.OPTIONAL, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING , true))); - tmpMap.put(_Fields.TIMESTAMP, new org.apache.thrift.meta_data.FieldMetaData("timestamp", org.apache.thrift.TFieldRequirementType.REQUIRED, + tmpMap.put(_Fields.TIMESTAMP, new org.apache.thrift.meta_data.FieldMetaData("timestamp", org.apache.thrift.TFieldRequirementType.OPTIONAL, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.I64))); tmpMap.put(_Fields.TTL, new org.apache.thrift.meta_data.FieldMetaData("ttl", org.apache.thrift.TFieldRequirementType.OPTIONAL, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.I32))); @@ -153,15 +153,10 @@ public class Column implements org.apach } public Column( - ByteBuffer name, - ByteBuffer value, - long timestamp) + ByteBuffer name) { this(); this.name = name; - this.value = value; - this.timestamp = timestamp; - setTimestampIsSet(true); } /** @@ -415,8 +410,8 @@ public class Column implements org.apach return false; } - boolean this_present_timestamp = true; - boolean that_present_timestamp = true; + boolean this_present_timestamp = true && this.isSetTimestamp(); + boolean that_present_timestamp = true && that.isSetTimestamp(); if (this_present_timestamp || that_present_timestamp) { if (!(this_present_timestamp && that_present_timestamp)) return false; @@ -450,7 +445,7 @@ public class Column implements org.apach if (present_value) builder.append(value); - boolean present_timestamp = true; + boolean present_timestamp = true && (isSetTimestamp()); builder.append(present_timestamp); if (present_timestamp) builder.append(timestamp); @@ -566,9 +561,6 @@ public class Column implements org.apach iprot.readStructEnd(); // check for required fields of primitive type, which can't be checked in the validate method - if (!isSetTimestamp()) { - throw new org.apache.thrift.protocol.TProtocolException("Required field 'timestamp' was not found in serialized data! Struct: " + toString()); - } validate(); } @@ -582,13 +574,17 @@ public class Column implements org.apach oprot.writeFieldEnd(); } if (this.value != null) { - oprot.writeFieldBegin(VALUE_FIELD_DESC); - oprot.writeBinary(this.value); + if (isSetValue()) { + oprot.writeFieldBegin(VALUE_FIELD_DESC); + oprot.writeBinary(this.value); + oprot.writeFieldEnd(); + } + } + if (isSetTimestamp()) { + oprot.writeFieldBegin(TIMESTAMP_FIELD_DESC); + oprot.writeI64(this.timestamp); oprot.writeFieldEnd(); } - oprot.writeFieldBegin(TIMESTAMP_FIELD_DESC); - oprot.writeI64(this.timestamp); - oprot.writeFieldEnd(); if (isSetTtl()) { oprot.writeFieldBegin(TTL_FIELD_DESC); oprot.writeI32(this.ttl); @@ -610,18 +606,22 @@ public class Column implements org.apach org.apache.thrift.TBaseHelper.toString(this.name, sb); } first = false; - if (!first) sb.append(", "); - sb.append("value:"); - if (this.value == null) { - sb.append("null"); - } else { - org.apache.thrift.TBaseHelper.toString(this.value, sb); + if (isSetValue()) { + if (!first) sb.append(", "); + sb.append("value:"); + if (this.value == null) { + sb.append("null"); + } else { + org.apache.thrift.TBaseHelper.toString(this.value, sb); + } + first = false; + } + if (isSetTimestamp()) { + if (!first) sb.append(", "); + sb.append("timestamp:"); + sb.append(this.timestamp); + first = false; } - first = false; - if (!first) sb.append(", "); - sb.append("timestamp:"); - sb.append(this.timestamp); - first = false; if (isSetTtl()) { if (!first) sb.append(", "); sb.append("ttl:"); @@ -637,10 +637,6 @@ public class Column implements org.apach if (name == null) { throw new org.apache.thrift.protocol.TProtocolException("Required field 'name' was not present! Struct: " + toString()); } - if (value == null) { - throw new org.apache.thrift.protocol.TProtocolException("Required field 'value' was not present! Struct: " + toString()); - } - // alas, we cannot check 'timestamp' because it's a primitive and you chose the non-beans generator. } private void writeObject(java.io.ObjectOutputStream out) throws java.io.IOException { Modified: cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Constants.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Constants.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Constants.java (original) +++ cassandra/branches/cassandra-0.8/interface/thrift/gen-java/org/apache/cassandra/thrift/Constants.java Sun Apr 17 05:10:17 2011 @@ -44,6 +44,6 @@ import org.slf4j.LoggerFactory; public class Constants { - public static final String VERSION = "20.1.0"; + public static final String VERSION = "20.2.0"; } Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cli/CliClient.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cli/CliClient.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cli/CliClient.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cli/CliClient.java Sun Apr 17 05:10:17 2011 @@ -802,7 +802,7 @@ public class CliClient if(superColumnName != null) parent.setSuper_column(superColumnName); - Column columnToInsert = new Column(columnName, columnValueInBytes, FBUtilities.timestampMicros()); + Column columnToInsert = new Column(columnName).setValue(columnValueInBytes).setTimestamp(FBUtilities.timestampMicros()); // children count = 3 mean that we have ttl in arguments if (statement.getChildCount() == 3) Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g Sun Apr 17 05:10:17 2011 @@ -382,11 +382,9 @@ K_FROM: F R O M; K_WHERE: W H E R E; K_AND: A N D; K_KEY: K E Y; -K_COLUMN: C O L (U M N)?; K_INSERT: I N S E R T; K_UPDATE: U P D A T E; K_WITH: W I T H; -K_ROW: R O W; K_LIMIT: L I M I T; K_USING: U S I N G; K_CONSISTENCY: C O N S I S T E N C Y; Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java Sun Apr 17 05:10:17 2011 @@ -470,12 +470,15 @@ public class QueryProcessor CqlResult result = new CqlResult(); logger.debug("CQL statement type: {}", statement.type.toString()); + CFMetaData metadata; + AbstractType<?> comparator; switch (statement.type) { case SELECT: SelectStatement select = (SelectStatement)statement.statement; clientState.hasColumnFamilyAccess(select.getColumnFamily(), Permission.READ); - validateColumnFamily(keyspace, select.getColumnFamily(), false); + metadata = validateColumnFamily(keyspace, select.getColumnFamily(), false); + comparator = metadata.getComparatorFor(null); validateSelect(keyspace, select); List<org.apache.cassandra.db.Row> rows = null; @@ -519,19 +522,8 @@ public class QueryProcessor /// No results for this row if (row.cf == null) continue; - - List<Column> thriftColumns = new ArrayList<Column>(); - for (IColumn column : row.cf.getSortedColumns()) - { - if (column.isMarkedForDelete()) - continue; - Column c = new Column(); - c.name = column.name(); - c.value = column.value(); - c.timestamp = column.timestamp(); - thriftColumns.add(c); - } - + + List<Column> thriftColumns = extractThriftColumns(select, comparator, row); // Create a new row, add the columns to it, and then add it to the list of rows CqlRow cqlRow = new CqlRow(); cqlRow.key = row.key.key; @@ -592,8 +584,8 @@ public class QueryProcessor case DELETE: DeleteStatement delete = (DeleteStatement)statement.statement; clientState.hasColumnFamilyAccess(delete.getColumnFamily(), Permission.WRITE); - CFMetaData metadata = validateColumnFamily(keyspace, delete.getColumnFamily(), false); - AbstractType<?> comparator = metadata.getComparatorFor(null); + metadata = validateColumnFamily(keyspace, delete.getColumnFamily(), false); + comparator = metadata.getComparatorFor(null); AbstractType<?> keyType = DatabaseDescriptor.getCFMetaData(keyspace, delete.getColumnFamily()).getKeyValidator(); @@ -792,7 +784,44 @@ public class QueryProcessor return null; // We should never get here. } - + + private static List<Column> extractThriftColumns(SelectStatement select, AbstractType<?> comparator, Row row) + { + List<Column> thriftColumns = new ArrayList<Column>(); + if (select.isColumnRange()) + { + // preserve comparator order + for (IColumn c : row.cf.getSortedColumns()) + { + if (c.isMarkedForDelete()) + continue; + thriftColumns.add(new Column(c.name()).setValue(c.value()).setTimestamp(c.timestamp())); + } + } + else + { + // order columns in the order they were asked for + for (Term term : select.getColumnNames()) + { + ByteBuffer name; + try + { + name = term.getByteBuffer(comparator); + } + catch (InvalidRequestException e) + { + throw new AssertionError(e); + } + IColumn c = row.cf.getColumn(name); + if (c == null || c.isMarkedForDelete()) + thriftColumns.add(new Column().setName(name)); + else + thriftColumns.add(new Column(c.name()).setValue(c.value()).setTimestamp(c.timestamp())); + } + } + return thriftColumns; + } + private static CQLStatement getStatement(String queryStr) throws InvalidRequestException, RecognitionException { // Lexer and parser Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/hadoop/ColumnFamilyRecordWriter.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/hadoop/ColumnFamilyRecordWriter.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/hadoop/ColumnFamilyRecordWriter.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/hadoop/ColumnFamilyRecordWriter.java Sun Apr 17 05:10:17 2011 @@ -206,7 +206,7 @@ implements org.apache.hadoop.mapred.Reco private Column avroToThrift(org.apache.cassandra.hadoop.avro.Column acol) { - return new Column(acol.name, acol.value, acol.timestamp); + return new Column(acol.name).setValue(acol.value).setTimestamp(acol.timestamp); } /** Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/CassandraServer.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/CassandraServer.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/CassandraServer.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/CassandraServer.java Sun Apr 17 05:10:17 2011 @@ -146,7 +146,7 @@ public class CassandraServer implements { continue; } - Column thrift_column = new Column(column.name(), column.value(), column.timestamp()); + Column thrift_column = new Column(column.name()).setValue(column.value()).setTimestamp(column.timestamp()); if (column instanceof ExpiringColumn) { thrift_column.setTtl(((ExpiringColumn) column).getTimeToLive()); @@ -195,7 +195,7 @@ public class CassandraServer implements } else { - Column thrift_column = new Column(column.name(), column.value(), column.timestamp()); + Column thrift_column = new Column(column.name()).setValue(column.value()).setTimestamp(column.timestamp()); if (column instanceof ExpiringColumn) { thrift_column.setTtl(((ExpiringColumn) column).getTimeToLive()); Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java Sun Apr 17 05:10:17 2011 @@ -379,6 +379,10 @@ public class ThriftValidation public static void validateColumnData(CFMetaData metadata, Column column) throws InvalidRequestException { validateTtl(column); + if (!column.isSetValue()) + throw new InvalidRequestException("Column value is required"); + if (!column.isSetTimestamp()) + throw new InvalidRequestException("Column timestamp is required"); try { AbstractType validator = metadata.getValueValidator(column.name); Modified: cassandra/branches/cassandra-0.8/test/system/test_cql.py URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/system/test_cql.py?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/test/system/test_cql.py (original) +++ cassandra/branches/cassandra-0.8/test/system/test_cql.py Sun Apr 17 05:10:17 2011 @@ -136,13 +136,16 @@ class TestCql(ThriftTester): def test_select_columns(self): "retrieve multiple columns" cursor = init() + # we deliberately request columns in non-comparator order cursor.execute(""" - SELECT 'cd1', 'col' FROM StandardString1 WHERE KEY = 'kd' + SELECT ca1, col, cd1 FROM StandardString1 WHERE KEY = 'kd' """) d = cursor.description - assert "cd1" in [col_dscptn[0] for col_dscptn in d] - assert "col" in [col_dscptn[0] for col_dscptn in d] + assert ['Row Key', 'ca1', 'col', 'cd1'] == [col_dscptn[0] for col_dscptn in d], d + row = cursor.fetchone() + # check that the column that didn't exist in the row comes back as null + assert ['kd', None, 'val', 'vd1'] == row, row def test_select_row_range(self): "retrieve a range of rows with columns" @@ -307,9 +310,8 @@ class TestCql(ThriftTester): cursor.execute(""" SELECT 'cd1', 'col' FROM StandardString1 WHERE KEY = 'kd' """) - colnames = [col_d[0] for col_d in cursor.description] - assert "cd1" in colnames - assert "col" in colnames + assert ['Row Key', 'cd1', 'col'] == [col_d[0] for col_d in cursor.description] + cursor.execute(""" DELETE 'cd1', 'col' FROM StandardString1 WHERE KEY = 'kd' """) @@ -317,7 +319,7 @@ class TestCql(ThriftTester): SELECT 'cd1', 'col' FROM StandardString1 WHERE KEY = 'kd' """) r = cursor.fetchone() - assert len(r) == 1 + assert ['kd', None, None] == r, r def test_delete_columns_multi_rows(self): "delete columns from multiple rows" @@ -325,22 +327,22 @@ class TestCql(ThriftTester): cursor.execute("SELECT 'col' FROM StandardString1 WHERE KEY = 'kc'") r = cursor.fetchone() - assert len(r) == 2 + assert ['kc', 'val'] == r, r cursor.execute("SELECT 'col' FROM StandardString1 WHERE KEY = 'kd'") r = cursor.fetchone() - assert len(r) == 2 + assert ['kd', 'val'] == r, r cursor.execute(""" DELETE 'col' FROM StandardString1 WHERE KEY IN ('kc', 'kd') """) cursor.execute("SELECT 'col' FROM StandardString1 WHERE KEY = 'kc'") r = cursor.fetchone() - assert len(r) == 1 + assert ['kc', None] == r, r cursor.execute("SELECT 'col' FROM StandardString1 WHERE KEY = 'kd'") r = cursor.fetchone() - assert len(r) == 1 + assert ['kd', None] == r, r def test_delete_rows(self): "delete entire rows" @@ -348,15 +350,13 @@ class TestCql(ThriftTester): cursor.execute(""" SELECT 'cd1', 'col' FROM StandardString1 WHERE KEY = 'kd' """) - colnames = [col_d[0] for col_d in cursor.description] - assert "cd1" in colnames - assert "col" in colnames + assert ['Row Key', 'cd1', 'col'] == [col_d[0] for col_d in cursor.description] cursor.execute("DELETE FROM StandardString1 WHERE KEY = 'kd'") cursor.execute(""" SELECT 'cd1', 'col' FROM StandardString1 WHERE KEY = 'kd' """) r = cursor.fetchone() - assert len(r) == 1 + assert ['kd', None, None] == r, r def test_create_keyspace(self): "create a new keyspace" Modified: cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/client/TestRingCache.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/client/TestRingCache.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/client/TestRingCache.java (original) +++ cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/client/TestRingCache.java Sun Apr 17 05:10:17 2011 @@ -103,7 +103,7 @@ public class TestRingCache // now, read the row back directly from the host owning the row locally tester.setup(firstEndpoint.getHostAddress(), DatabaseDescriptor.getRpcPort()); tester.thriftClient.set_keyspace(keyspace); - tester.thriftClient.insert(row, parent, new Column(ByteBufferUtil.bytes("col1"), ByteBufferUtil.bytes("val1"), 1), ConsistencyLevel.ONE); + tester.thriftClient.insert(row, parent, new Column(ByteBufferUtil.bytes("col1")).setValue(ByteBufferUtil.bytes("val1")).setTimestamp(1), ConsistencyLevel.ONE); Column column = tester.thriftClient.get(row, col, ConsistencyLevel.ONE).column; System.out.println("read row " + new String(row.array()) + " " + new String(column.name.array()) + ":" + new String(column.value.array()) + ":" + column.timestamp); } Modified: cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/service/EmbeddedCassandraServiceTest.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/service/EmbeddedCassandraServiceTest.java?rev=1094102&r1=1094101&r2=1094102&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/service/EmbeddedCassandraServiceTest.java (original) +++ cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/service/EmbeddedCassandraServiceTest.java Sun Apr 17 05:10:17 2011 @@ -84,8 +84,10 @@ public class EmbeddedCassandraServiceTes cp.column = ByteBufferUtil.bytes("name"); // insert - client.insert(key_user_id, par, new Column(ByteBufferUtil.bytes("name"), - ByteBufferUtil.bytes("Ran"), timestamp), ConsistencyLevel.ONE); + client.insert(key_user_id, + par, + new Column(ByteBufferUtil.bytes("name")).setValue(ByteBufferUtil.bytes("Ran")).setTimestamp(timestamp), + ConsistencyLevel.ONE); // read ColumnOrSuperColumn got = client.get(key_user_id, cp, ConsistencyLevel.ONE);