deanhiller / playorm

ORM for nosql with SCALABLE SQL
https://github.com/deanhiller/playorm/wiki
Other
76 stars 18 forks source link

@NoSqlEmbedded doesn't work with List and Set of Long types #122

Open hsn10 opened 11 years ago

hsn10 commented 11 years ago

----Changing the heading and description of this issue------------29/07/2013

List and Set are not working with @NoSqlEmbedded fields...

As embedded stuff is also stored in columnName, we are facing issue with composite types..The issue is similar to issue #67

-----Original issue (which is fixed for String etc)------ currently it works only for java.util.List

Its not clear from documentation if order is maintained within List and if duplicate values are allowed, it looks to me that it is implemented like Set, not List.

http://buffalosw.com/wiki/NoSqlEmbedded-for-Integer,-LocalDate-and-String-list/

In that case allowing Set is just matter of API.

hsn10 commented 11 years ago

i checked source code at MetaEmbeddedEntity and java.util.Set should work. I need to decompile scala code to see what type is being used. It probably would be HashSet

hsn10 commented 11 years ago

Caused by: java.lang.RuntimeException: Failure scanning field=private java.util. Set models.SetTest.notes for class=SetTest at com.alvazan.orm.impl.meta.scan.ScannerForClass.inspectField(ScannerFo rClass.java:216) at com.alvazan.orm.impl.meta.scan.ScannerForClass.scanFields(ScannerForC lass.java:209) Caused by: java.lang.IllegalArgumentException: We found no converters(customer o r standard for type=Set and this class is not annotated with @NoSqlEmbeddable ei ther. The field that caused this issue is field=private java.util.Set models.Se tTest.notes at com.alvazan.orm.impl.meta.scan.ScannerForField.lookupConverter(Scanne rForField.java:367) at com.alvazan.orm.impl.meta.scan.ScannerForField.processEmbedded(Scanne rForField.java:351)

hsn10 commented 11 years ago

Test code for this case is at https://github.com/hsn10/ormtest/tree/testset

easility commented 11 years ago

Set is not supported yet.

Also, if you are looking for Set of Long type elements then it is MetaEmbeddedSImple which is applicable not MetaEmbeddedEntity. MetaEmbeddedEntity will be applicable for Set of some entities.

hsn10 commented 11 years ago

saving set should work. MetaEmbeddedSimple. translateToColumn calls translateToColumnList which works with generic Collection. Is there way to debug what exactl string causes cassandra error?

deanhiller commented 11 years ago

I also thought Set worked. Vikas, let me know what the issue was as this one I thought always worked. (make sure we have a test case for it too).

hsn10 commented 11 years ago

reading from set (deserialization) is not yet supported MetaEmbeddedSimple.translateFromColumnSet(Row row, OWNER entity, ...) is doing nothing.

deanhiller commented 11 years ago

I sent vikas an email on what code to look at as it looks pretty simple to add as it turned out the Set I was thinking of was in a *ToMany situation where it is supported. We just need to extend one class with 2 or 3 methods and hardly any code in those as the collection class we have has all the core logic which is shared between Set and List.

easility commented 11 years ago

This is done now. Check in the latest Code. closing

hsn10 commented 11 years ago

there is problem with serializing Set if set is not String type

hsn10 commented 11 years ago

root cause is comparator in table definition. Its set to UTF8Type, so you can't store binary values (Long) into column names. Test code for this is at https://github.com/hsn10/ormtest/tree/testset

create column family SetTest with column_type = 'Standard' and comparator = 'UTF8Type' and default_validation_class = 'BytesType' and key_validation_class = 'IntegerType' and read_repair_chance = 0.1 and dclocal_read_repair_chance = 0.0 and gc_grace = 864000 and min_compaction_threshold = 4 and max_compaction_threshold = 32 and replicate_on_write = true and compaction_strategy = 'org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy' and caching = 'KEYS_ONLY' and compression_options = {'sstable_compression' : 'org.apache.cassandra.io.compress.SnappyCompressor'};

easility commented 11 years ago

I change the Set to type long and it worked.. please see this test https://github.com/deanhiller/playorm/commit/f7ff0044698369198d63a7c89b06f4f95c2178df

easility commented 11 years ago

I think the problem is that you will have to let playorm create the ColumFamily for you in Cassandra.

hsn10 commented 11 years ago

it depends what Long value you are inserting into set. if cassandra can parse value as valid utf8 string then it works. I am inserting Random.nextLong(). Thats why is using randomized testing like - http://labs.carrotsearch.com/randomizedtesting.html

easility commented 11 years ago

hmm..Ok. I ran your test against InMemory and MongoDB and it passed there too with that much long number.. it seems that it is an Astynax issue... Also, I am not pulling your request in the master as it will break other's build with Cassandra.

hsn10 commented 11 years ago

it should create CF with Composite comparator if @NosqlEmbedded is used.

easility commented 11 years ago

Not sure if it would help. We tried to change the Comparator in case of issue #67 too which is related to similar issue but failed to do so. It seems that in light of NoSqlEmbedded we would again need to pick it up.

easility commented 11 years ago

re-opening it as this defect is coming for type long as well

hsn10 commented 11 years ago

I am looking at source code. What about to add field "has embedded CF" to DboTableMeta and set it to true if there is something @NoSqlEmbedded in entity class. If this is true then ColumnType must be composite

easility commented 11 years ago

What composite type it should be? I tried giving CompositeType(UTF8Type, UTF8Type) and CompositeType(UTF8Type, BytesType)

but in both cases it gave following error:

java.lang.RuntimeException: com.netflix.astyanax.connectionpool.exceptions.BadRequestException: BadRequestException: [host=127.0.0.1(127.0.0.1):9160, latency=2(2), attempts=1]InvalidRequestException(why:Not enough bytes to read value of component 0) at com.alvazan.orm.layer9z.spi.db.cassandra.CassandraSession.sendChanges(CassandraSession.java:116) at com.alvazan.orm.logging.NoSqlRawLogger.sendChanges(NoSqlRawLogger.java:52) at com.alvazan.orm.layer5.nosql.cache.NoSqlWriteCacheImpl.flush(NoSqlWriteCacheImpl.java:138) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197) Caused by: com.netflix.astyanax.connectionpool.exceptions.BadRequestException: BadRequestException: [host=127.0.0.1(127.0.0.1):9160, latency=2(2), attempts=1]InvalidRequestException(why:Not enough bytes to read value of component 0) at com.netflix.astyanax.thrift.ThriftConverter.ToConnectionPoolException(ThriftConverter.java:159) at com.netflix.astyanax.thrift.AbstractOperationImpl.execute(AbstractOperationImpl.java:65) at com.netflix.astyanax.thrift.AbstractOperationImpl.execute(AbstractOperationImpl.java:28) at com.netflix.astyanax.thrift.ThriftSyncConnectionFactoryImpl$ThriftConnection.execute(ThriftSyncConnectionFactoryImpl.java:151) at com.netflix.astyanax.connectionpool.impl.AbstractExecuteWithFailoverImpl.tryOperation(AbstractExecuteWithFailoverImpl.java:69) at com.netflix.astyanax.connectionpool.impl.AbstractHostPartitionConnectionPool.executeWithFailover(AbstractHostPartitionConnectionPool.java:256) at com.netflix.astyanax.thrift.ThriftKeyspaceImpl.executeOperation(ThriftKeyspaceImpl.java:485) at com.netflix.astyanax.thrift.ThriftKeyspaceImpl.access$000(ThriftKeyspaceImpl.java:79) at com.netflix.astyanax.thrift.ThriftKeyspaceImpl$1.execute(ThriftKeyspaceImpl.java:123) at com.alvazan.orm.layer9z.spi.db.cassandra.CassandraSession.sendChangesImpl(CassandraSession.java:139) at com.alvazan.orm.layer9z.spi.db.cassandra.CassandraSession.sendChanges(CassandraSession.java:114) ... 31 more Caused by: InvalidRequestException(why:Not enough bytes to read value of component 0) at org.apache.cassandra.thrift.Cassandra$batch_mutate_result.read(Cassandra.java:20833) at org.apache.thrift.TServiceClient.receiveBase(TServiceClient.java:78) at org.apache.cassandra.thrift.Cassandra$Client.recv_batch_mutate(Cassandra.java:964) at org.apache.cassandra.thrift.Cassandra$Client.batch_mutate(Cassandra.java:950) at com.netflix.astyanax.thrift.ThriftKeyspaceImpl$1$1.internalExecute(ThriftKeyspaceImpl.java:129) at com.netflix.astyanax.thrift.ThriftKeyspaceImpl$1$1.internalExecute(ThriftKeyspaceImpl.java:126) at com.netflix.astyanax.thrift.AbstractOperationImpl.execute(AbstractOperationImpl.java:60) ... 40 more

hsn10 commented 11 years ago

CompositeType(UTF8Type, BytesType). First component would be name of @NoSqlEmbedded column, second set content

easility commented 11 years ago

That too didn't work.. :( and I got same error as my previous comment

hsn10 commented 11 years ago

can you push your branch somewhere?

easility commented 11 years ago

I deleted that code..but it was not a major change..basically we are setting the comparator at setColumnNameCompareType() method in ColumnFamilyHelper....

hsn10 commented 11 years ago

There are 2 ways how to make this working.

  1. Redefine all CF to comparator CompositeType(UTF8Type, BytesType)
  2. add meta flag if embedded elements are then use composite comparator otherwise use standard utf8.
hsn10 commented 11 years ago

Bug is not specific to Set any embeddable with high number fails.

hsn10 commented 11 years ago

This is serious bug because you can't store in embedded entity anything which do not validates as utf8.

3rd possible workaround is just define comparator as byte[].

deanhiller commented 11 years ago

UTF8Type, BytesType is correct as for embedded as you said. I have had not time to look into this. This exception InvalidRequestException(why:Not enough bytes to read value of component 0) is usually a mistake in the entity we are using to save the data....it must match String and byte[] or you get that exception....at least if I remember correctly.

deanhiller commented 11 years ago

I should not say a mistake in the entity. I should have said a mistake in a lower level pojo that we have to feed in to astyanax or you get this weird exception. I have seen that exception previously as well.

hsn10 commented 11 years ago

what method did you choose for fixing this? Redefine all columns as composite, no matter if embedded entity are used?

deanhiller commented 11 years ago

The method I think we plan to use to fix this is CF's should be created as composite UTF8 / BytesType....we may have to add to existing annotation @NoSqlEmbedded(legacy=true) to make sure any existing CF and code out there does not break as well which would continue to translate previous types.

hsn10 commented 11 years ago

Did you tried to change comparator to uf8/bytes and read data back?