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.

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

SQLite dialect #1865

Closed
wants to merge 13 commits into from
Closed

SQLite dialect #1865

wants to merge 13 commits into from

Conversation

luca-vercelli
Copy link

As discussed here, I have updated gradle configuration for including SQLite dialect.
Out of 369 tests, 28 fails and 42 are ignored, with a 91% of success. I guess tests fail due to JDBC limitations. Here is the list of failing tests:

BatchTest. testScroll
BatchTest. testStatelessSession
AutoFlushTest. testFlushAutoSQL
AutoFlushTest. testFlushAutoSQLNativeSession
AutoFlushTest. testFlushAutoSQLSynchronization
HQLTest. test_hql_all_subquery_comparison_qualifier_example
HQLTest. test_hql_api_scroll_open_example
HQLTest. test_hql_collection_expressions_example_5
HQLTest. test_hql_collection_expressions_example_7
HQLTest. test_hql_collection_expressions_example_8
HQLTest. test_hql_extract_function_example
HQLTest. test_hql_in_predicate_example_6
HQLTest. test_hql_locate_function_example
HQLTest. test_hql_numeric_arithmetic_example_2
HQLTest. test_hql_numeric_arithmetic_example_3
HQLTest. test_hql_substring_function_example
HQLTest. test_hql_year_function_example
BlobByteArrayTest. test
BlobTest. test
ClobCharArrayTest. test
ClobStringTest. test
ClobTest. test
NClobCharArrayTest. test
NClobStringTest. test
NClobTest. test
NationalizedTest. test
DatabaseValueGenerationTest. test
PooledOptimizerTest. test

@vladmihalcea
Copy link
Contributor

Thanks @luca-vercelli. I'll review on Monday.

@vladmihalcea
Copy link
Contributor

vladmihalcea commented Apr 3, 2017

@luca-vercelli Did you sign the CLA?

@vladmihalcea
Copy link
Contributor

You have a merge commit which prevents me from rebasing your work on top of master. Try squashing all your changes and submit it as one commit so that I can integrate it. We never issue any merge between branches. We always rebase.


@Override
public boolean hasAlterTable() {
// As specified in NHibernate dialect
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this imported from NHibernate? Can we check it how it works there too?

@Override
public boolean hasDataTypeInIdentityColumn() {
// As specified in NHibernate dialect
// FIXME true
Copy link
Contributor

Choose a reason for hiding this comment

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

How should this be fixed in future. Is this also something related to NHibernate?

build.gradle Outdated
@@ -169,6 +169,12 @@ subprojects { subProject ->
testRuntime( libraries.mssql )
testRuntime( libraries.informix )

if (db.equalsIgnoreCase("sqlite")) {
dependencies {
testRuntime( libraries.sqlite )
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not providing it by default? The Oracle is a special case because it's not available on Maven Central.

@vladmihalcea
Copy link
Contributor

@luca-vercelli I see you committed on master on your fork. This is not usually a good thing to do since it restricts you to fetching the latest changes from upstream.

What you can do to easily fix the merge commit problem is to follow these steps:

  1. Create a new branch called sqlite
  2. git checkout sqlite
  3. git merge --squash master

Then you should have all changes in the sqlite branch which you can use to send the PR.

The master branch should be reset so that all extra commits you've done are removed. Then you can merge from upstream/master, assuming you added the upstream mirror, and then you can also rebase the sqlite branch.

@luca-vercelli
Copy link
Author

@vladmihalcea, yes, I just begun to work on this. I will create a new branch ASAP.

@vladmihalcea
Copy link
Contributor

@luca-vercelli Thanks

@luca-vercelli
Copy link
Author

Dear @vladmihalcea your suggestion does not work. I did:

$git branch sqlite
$git checkout sqlite
$git merge --squash master
 (nothing to squash)Already up-to-date.

@luca-vercelli
Copy link
Author

Maybe I could just drop completely my repository, then create a new one again and put all gwenn's modified files as they were mine... I don't like that so much...

@vladmihalcea
Copy link
Contributor

You can also use git cherry-pick each commit that you did on master onto the new branch. Make sure that you skip the merge commits.

@luca-vercelli
Copy link
Author

You should find a new branch sqlite. I have git cherry-picked all commits but merge commits into that branch.

@vladmihalcea
Copy link
Contributor

Thanks. I'll check it out on Monday.

@vladmihalcea
Copy link
Contributor

Replaced by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants