search

scassandra / scassandra-server

Stubbed Cassandra
http://www.scassandra.org
Other
87 stars 34 forks source link

Adds support for statically typed insert statements. #133

Closed MicahZoltu closed 8 years ago

MicahZoltu commented 8 years ago

See the new test in PreparedStatementsTest for an example of the problem being fixed. Previously the test would fail on the call to setLong because the prepared statement result from the stub server did not contain column type information.

Fixes #132

This is not ready for merge, but I am a bit stuck. The original bug has been fixed, but some tests are now failing and I don't fully understand why (nor do I understand what they were testing). In particular, it appears the column_types were working for select statements and now they aren't. By not sending back column information in the prepare result (just commenting out the columns.forEach) fixes the old tests... but of course breaks the new test.

MicahZoltu commented 8 years ago

I have fixed the failing tests by changing the PreparedResult to include the columns only if there are no primed variableTypes.

This seems pretty terrible to me, but I'm beginning to suspect fully supporting priming columns in both ways together would require some architectural changes or at the least a breaking change in the API. If anyone has any better ideas on how to accomplish this please let me know.

MicahZoltu commented 8 years ago

I have added support for asserting on activity. I am still very unhappy with this solution, but it gets the job done and fixes my use case. I would love to see a better solution, but I really do need something that works more than I need clean code at this juncture. :/

MicahZoltu commented 8 years ago

Bleh. My fix for asserting on activity doesn't actually work because the map.values() doesn't give the results in a well defined order, so you can't reliably assert on it. :(

MicahZoltu commented 8 years ago

I am now using this branch for my Cassandra testing since, even with the caveats, it is better than nothing. What are the chances of this getting merged into mainline? If they are low, then I'll look into making my fork a bit more permanent (setup CI, etc.). If they are high I'll continue waiting.

chbatey commented 8 years ago

thanks for your work on this @Zoltu I should have time to look at this properly at the weekend

chbatey commented 8 years ago

Closing this per the comment on the issue