Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add bind parameters support for cassandra native queries #983

Merged
merged 6 commits into from
Feb 27, 2018
Merged

Add bind parameters support for cassandra native queries #983

merged 6 commits into from
Feb 27, 2018

Conversation

AntonYudin
Copy link
Contributor

Adding bind parameters support for cassandra native queries.
In the future parts of this code could be used to re-implement regular JPQL queries to use bind parameters.

@AntonYudin
Copy link
Contributor Author

Hmm.
I cannot find on How to Contribute any references to "Codacy" requirements.

  1. I'm about to commit changes and change the pull request to move fields declarations.
  2. The complaint about the long method (three parameters) is strange, because this same file has much longer methods and constructors.

@AntonYudin
Use JPA compatible parameter indexing - indexing starts with 1. Cassa…
938be99 
…ndra DataStax driver parameter indexing starts with 0.
@karthikprasad13
Copy link
Collaborator

@AntonYudin

Please add formatting and valid test cases for the same code.

You can use Kundera Formatter for formatting the code.

-Karthik

@AntonYudin
Copy link
Contributor Author

AntonYudin commented Feb 13, 2018 •
edited

While working on the tests, I found out that some tests that have nothing to do with my changes in are failing.
Is that expected?

PersonCassandraTest
CassandraIdQueryTest
CassandraNativeFunctionsTest

@devender-yadav
Copy link
Contributor

Hi @AntonYudin,

No, it should not fail. What Cassandra version are you using? Can you share root cause of the test case failure?

@AntonYudin
Copy link
Contributor Author

I'm running apache-cassandra-3.11.0
Here is one:

------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.impetus.client.cassandra.config.CassandraPropertySetterTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 256.352 sec
Running com.impetus.client.cassandra.config.CassandraSchemaGenerationUsingXmlTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 147.252 sec
Running com.impetus.client.cassandra.config.CassandraUserTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 166.272 sec
Running com.impetus.client.cassandra.query.CassQueryTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 128.727 sec
Running com.impetus.client.cassandra.query.JpaQueryTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.226 sec
Running com.impetus.client.cassandra.thrift.cql.CassandraBatchProcessorCQLTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 216.241 sec
Running com.impetus.client.cassandra.thrift.cql.CassandraCqlSecondaryTableTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 33.697 sec
Running com.impetus.client.cassandra.thrift.cql.CassandraShowQueryTest
CREATE KEYSPACE IF NOT EXISTS "showQueryKeyspace" WITH REPLICATION = { 'class':'org.apache.cassandra.locator.SimpleStrategy','replication_factor':1} and durable_writes = 'true'
CREATE COLUMNFAMILY "USER" ("name" text ,"age" int ,"id" int  , PRIMARY KEY("id"))
create index if not exists on "USER"("name")
create index if not exists on "USER"("age")
CREATE COLUMNFAMILY "mobile_handset" ("mob_name" text ,"os" text ,"manufacturer" text ,"id" text  , PRIMARY KEY("id"))
create index if not exists on "mobile_handset"("os")
create index if not exists on "mobile_handset"("manufacturer")
CREATE COLUMNFAMILY "PersonChild" ("lastName" text ,"firstName" text ,"id" text  , PRIMARY KEY("id"))
CREATE COLUMNFAMILY "Person" ("lastName" text ,"firstName" text ,"id" text  , PRIMARY KEY("id"))
CREATE COLUMNFAMILY "operating_system" ("os_name" text ,"os_id" text  , PRIMARY KEY("os_id"))
CREATE COLUMNFAMILY "mobile_manufacturer" ("app_name" text ,"app_id" text  , PRIMARY KEY("app_id"))
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 103.804 sec
Running com.impetus.client.cassandra.thrift.cql.CQLUserTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 221.518 sec
Running com.impetus.client.cassandra.thrift.cql.LoggingConfigurationTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 180.447 sec
Running com.impetus.client.cassandra.thrift.cql.OTMCRUDCQLTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 246.38 sec
Running com.impetus.client.cassandra.thrift.cql.PersonCassandraCQLTest
Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 262.935 sec <<< FAILURE!
onInsertCassandra(com.impetus.client.cassandra.thrift.cql.PersonCassandraCQLTest)  Time elapsed: 38.335 sec  <<< FAILURE!
junit.framework.AssertionFailedError: null
	at junit.framework.Assert.fail(Assert.java:47)
	at junit.framework.Assert.assertTrue(Assert.java:20)
	at junit.framework.Assert.assertFalse(Assert.java:34)
	at junit.framework.Assert.assertFalse(Assert.java:41)
	at com.impetus.client.crud.BaseTest.assertFindByRange(BaseTest.java:352)
	at com.impetus.client.crud.PersonCassandraTest.onInsertCassandra(PersonCassandraTest.java:189)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
	at org.junit.runners.BlockJUnit4ClassRunner.runNotIgnored(BlockJUnit4ClassRunner.java:79)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:71)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:49)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
	at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
	at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)

@AntonYudin
Copy link
Contributor Author

This is really strange. I got that after running mvn clean package
Tests run: 228, Failures: 22, Errors: 6, Skipped: 0
I'm attaching a full build log.
build.log

@karthikprasad13
Copy link
Collaborator

@AntonYudin

Above test cases failed while testing range queries. You need to change Cassandra's default partitioner to ByteOrderedPartitioner for these to work.

-Karthik

@AntonYudin
Copy link
Contributor Author

Okay. After a fresh clone of the kundera's source code and fresh install of cassandra 3.11.1 with the partitioner set to ByteOrderedPartitioner the tests are failing with the same error messages.

I looked at the first failed test (CassandraNativeFunctionsTest.java). The test expects 30 and gets 500 for MAX(AGE). There is another test method in this test class that actually populates the data with the age 500!

So depending on the tests order, the test could fail or be successful.

By default the order of the test methods is not known. It could be anything depending on the environment.

According to JUnit, test method should be designed in a way that does not make them dependent on the order they are ran.

JUnit 4.11 has a way to specify the order: FixMethodOrder.

I'm looking at whether that will help with the first failed test.

I'm using oracle's jdk1.8.0_152 and maven-3.5.2

@AntonYudin
Copy link
Contributor Author

After fixing the CassandraNativeFunctionsTest.java, now I get only 9 errors for the cassandra-core module.
And most of those exceptions are org.apache.thrift.transport.TTransportException.
I'm including the build.log
build.log

@AntonYudin
Copy link
Contributor Author

So I added the tests.
The next step is to format the code to your standards.
Is filedropper.com the only way to do it?

Thanks.

@devender-yadav
Copy link
Contributor

@AntonYudin,

That filedropper link is probably dead. Try this:

https://github.com/devender-yadav/kundera-formatter/blob/master/cassandra_code_style.xml.zip?raw=true

Testcase order should not matter with existing test cases of kundera-cassandra module as far as I know. I will rebuild with a fresh kundera clone and cassandra at my end on Monday.

Thanks for the regular updates!

@devender-yadav
Copy link
Contributor

Also, avoid unused imports. Check codacy report for details.

https://app.codacy.com/app/devender-yadav/Kundera/pullRequest?prid=1287967

@AntonYudin
Copy link
Contributor Author

AntonYudin commented Feb 18, 2018 via email

@AntonYudin
Copy link
Contributor Author

AntonYudin commented Feb 18, 2018 via email

@AntonYudin
Copy link
Contributor Author

What is the status of reviewing my changes?
Is there anything else I can do to speed up the process of accepting these changes?
Thanks.

karthikprasad13
karthikprasad13 reviewed Feb 27, 2018
View reviewed changes
Copy link
Collaborator

@karthikprasad13 karthikprasad13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting is still not consistent with Kundera formatter. Logic and code looks fine

@karthikprasad13 karthikprasad13 merged commit dbce4e7 into Impetus:trunk Feb 27, 2018
@karthikprasad13
Copy link
Collaborator

@AntonYudin

We are verifying build locally, will format it at our end and push the code

@devender-yadav devender-yadav mentioned this pull request May 22, 2018
Closed
@devender-yadav devender-yadav added this to the Kundera 3.13 milestone May 22, 2018
@ccarpenter04 ccarpenter04 mentioned this pull request Oct 25, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karthikprasad13 karthikprasad13 karthikprasad13 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Kundera 3.13
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AntonYudin @karthikprasad13 @devender-yadav