Accept operator @@ to be used in the FullTextSearch. Find FieldIndex where columna name use _ #116

jjchiw · Dec 6, 2013

The @@ operator was interpreted as parameter....

var query = @"select * 
                     from posts 
                     where search_vector @@ to_tsquery('english', @p0) 
                     order by ts_rank_cd(search_vector, to_tsquery('english', @p0)) desc";
var p = "postgres";
var posts = ctx.Posts.SqlQuery(query, p).ToList();

Look for columns name in the format with underscore like created_at, first_name, etc...

Example: where search_vector @@ to_tsquery('english', @p0)

CreatedAt (field) == created_at (column name)

franciscojunior · Dec 9, 2013

Hi, @jjchiw !

I'm reviewing your patch here.

Would it be possible for you to include some unit tests? This would help us very much!

Thank you in advance.

jjchiw · Dec 10, 2013

Hi!

Yes sure, I can create the UnitTest I just have one doubt, this changes are for EntityFramework (right now using 6.0)

Do I create a new project, or include the dependencies in the TestProject.....

roji · Dec 10, 2013

Feel free to add a dependency from the NpgsqlTests project to the Npgsql.EntityFramework project - this will allow you to reference the EF6 classes.

Please put the EF-related tests in a new source file, just to keep things organized...

jjchiw · Dec 12, 2013

I did the tests...

But I'm not sure about adding NpgslTests project to Npgsql.EntityFramework.....anyway I just did that, and I had to sign the NpgsqlTests project in order to be used in Npgsql.EntityFramework project, maybe I didn't understad the comment of @roji

I just did the tests with postgresql 9.3.2 / .net 4.5

roji · Dec 12, 2013

@jjchiw, sorry, I think I wasn't very clear (and you sensed that)...

All tests in the solution should be in the NpgsqlTests project. What I meant was that there's no problem for that issue to depend on the Npgsql.EntityFramework project (and not vice versa), and to contain tests for EF. Within NpgsqlTests, it's a good idea to put tests in EntityFrameworkTests.cs (like you have done) to cleanly separate them from the other tests...

jjchiw · Dec 13, 2013

Ok, I think I got it this time.

roji · Dec 13, 2013

Looks much better @jjchiw, thanks.

However, I can see you added a reference in NpgsqlTests.csproj to a private path for Npgsql.EntityFramework.dll, this should be a project reference instead (just like a reference from NpgsqlTests to Npgsql).

Also, is the EntityFramework.SqlServer reference intended? Do we really need functionality from an SqlServer assembly?

@franciscojunior, apart from my external remarks I didn't really review this, so after the above is fixed it's up to you to merge :)

Removing EntityFramework.SqlServer from NpgsTests

jjchiw · Dec 13, 2013

Fixed the references...

EntityFramework.SqlServer is added automatically by nuget when you install EntityFramework....

I just removed it, clean the config file and it worked...

Thanks @roji for the assistance.

roji · Dec 13, 2013

Thanks @jjchiw, looks great. @franciscojunior, up to you now...

franciscojunior · Dec 13, 2013

Hi @jjchiw .

I noticed that there is a file App.config in the Npgsql.EntityFramework folder. Maybe it was left there configuring the tests/App.config file. I think there is no need for the App.config file inside the Npgsql.EntityFramework.

Removing App.config from Npgsql.EntityFramework

jjchiw · Dec 13, 2013

@franciscojunior

Sorry, from now on I'll try to be more careful. I just deleted it.....

franciscojunior · Dec 13, 2013

Sorry, from now on I'll try to be more careful. I just deleted it.....

Don't worry. There is no need to be sorry. That's why we are reviewing your pull request. :)

franciscojunior · Dec 13, 2013

Another question, after you changed the way to reference the Npgsql.EntityFramework project, do you still need to sign the tests? I think it would be better if we could get it to work without needing to sign it. What do you think?

jjchiw · Dec 13, 2013

Yes I'm agree with you.

I already removed the signature key from the signing tab in the Npgsql.Test
project

On Fri, Dec 13, 2013 at 7:13 PM, Francisco Figueiredo Jr. <
notifications@github.com> wrote:

Another question, after you changed the way to reference the
Npgsql.EntityFramework project, do you still need to sign the tests? I
think it would be better if we could get it to work without needing to sign
it. What do you think?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/116#issuecomment-30530629
.

franciscojunior · Dec 13, 2013

Yes I'm agree with you.

Great!

Would you mind to remove the key file from the tests folder?

I think you will need to commit your changes to the NpgsqlTests.csproj file so the reference to the snk file is removed.

I hope I'm not being too much annoying. :)

jjchiw · Dec 13, 2013

No problemo....

Ok, it's done the key file has been removed....

franciscojunior · Dec 13, 2013

No problemo....

Ok, it's done the key file has been removed....

Great! I'm doing some tests here.

franciscojunior · Dec 13, 2013

Just a little catch I got here when running the tests:

If you have Npgsql registered in the machine.config, you will get an error when running the tests because the registration of the DbProviderFactory is unique.

Thanks to David Karlas sample: #91 (comment) this has an easy fix. Just add the remove tag before the dbproviderfactory registration:

<system.data>
        <DbProviderFactories>
            <remove invariant="Npgsql" />
            <add name="Npgsql Data Provider" invariant="Npgsql" support="FF" description=".Net Framework Data Provider for Postgresql" type="Npgsql.NpgsqlFactory, Npgsql" />
        </DbProviderFactories>
    </system.data>

jjchiw · Dec 13, 2013

Oki......remove tag added...

On Fri, Dec 13, 2013 at 9:33 PM, Francisco Figueiredo Jr. <
notifications@github.com> wrote:

Just a little catch I got here when running the tests:

If you have Npgsql registered in the machine.config, you will get an error
when running the tests because the registration of the DbProviderFactory is
unique.

Thanks to David Karlas sample: #91 (comment)#91 (comment) has an easy fix. Just add the remove tag before the dbproviderfactory
registration:

<system.data>

</system.data>

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/116#issuecomment-30541429
.

franciscojunior · Dec 14, 2013

Hi, @jjchiw . I think your pull request is in good shape for merging.

I just didn't quite understand the part about the underscore of the parameter names. Does EF use some special formatting for parameter names? Can you give us more information about this process? I'm kind of new to EF internals and your comments would give us a lot of useful information about EF and our interaction with it.

Thanks in advance.

roji · Dec 23, 2013

@jjchiw, the best way to clean this up is to use git rebase --interactive. This will allow you to:

Remove commits which you want to get rid of (instead of adding revert commits, ilke for the #if WINDOWS)
Squash (merge) together commits
Base the pull request on the latest master

When you're doing with the rebasing on your local machine, do a push -f to force-push it to the github PR. This will effectively replace all the commits in this PR with the new rewritten ones.

This process will spare us from deleting the PR and creating a new one, keeping the same PR. Let me know if you need help with this process.

jjchiw · Dec 23, 2013

Ok, I think I did the rebase correctly, but now all the Tests fails first for this commit 6e32569 (I already fix this adding Npgsql.* to all the resources in NpgsqlProviderManifest class

The @@ operator has not been pulled in the master branch (Do I merge the changes from @glenebob branch, and commit it)

And the test of the underscore I'll just deleted it.....

roji · Dec 23, 2013

Ah, your PR depends on another branch/PR...

In that case you should probably rebase your PR on top of Glen's - since you depend on his commit. Then, after Glen's branch is merged your PR can be merged as well.

Example: where search_vector @@ to_tsquery('english', @p0)

CreatedAt (field) == created_at (column name)

Removing EntityFramework.SqlServer from NpgsTests

Removing App.config from Npgsql.EntityFramework

This reverts commit af0548a.

Conflicts: Npgsql/Npgsql/NpgsqlRowDescription.cs tests/EntityFrameworkTests.cs

glenebob · Dec 23, 2013

Another trick that's very useful is to create a branch in your own repo for each distinct piece of functionality you want to work on, and then offer a PR from your branches against master. That way you can have one or more PR open at a time, and also be able to do experimental work without having it effect any of the PR's you have open.

-Glen

glenebob · Mar 29, 2014

Hi @jjchiw,

Would it be possible for you to submit some tests for the three operators, @@, @>, and <@, isolated to not depend on EF in any way? We would want them to be as simple as possible. I just realized we still don't have these tests, so I'm not able to thoroughly test my fix for #193.

Thanks for any help you can offer here!

-Glen

Test Operators @> <@ @@

jjchiw · Mar 31, 2014

Added tests for operators @@ @> <@ in CommandTests and in EntityFrameworkBasicTests.

roji · Aug 9, 2015

Bumped into this after a long time while cleaning old issues... Seems still relevant. Does anyone want to pick this up and integrate into develop?

Emill · Aug 9, 2015

This was fixed by my new lexer.
Automatically transforming CamelCase to underscore_case doesn't seem like something Npgsql should do.

roji · Aug 10, 2015

Ah, great - thanks. Agree about the camelcase transformation...

jjchiw added 2 commits December 6, 2013 18:58

Accept operator @@ for full text search queries

346e5cc

Example: where search_vector @@ to_tsquery('english', @p0)

Find FieldIndex where column name use _

f1f4149

CreatedAt (field) == created_at (column name)

jjchiw added 2 commits December 12, 2013 18:51

Issue #116 - Tests

f734a88

update from upstream - issue #116 Tests

ea76d68

jjchiw added 2 commits December 13, 2013 08:59

Merge remote-tracking branch 'upstream/master'

d60e12a

#116 Tests in NpgsqlTests Project

8be9316

#116 fixing references

b41e734

Removing EntityFramework.SqlServer from NpgsTests

#116 Removing App.config

3bdb8c1

Removing App.config from Npgsql.EntityFramework

#116 removing the key file from the tests folder

8b6638a

#116 Add to app.config <remove invariant="Npgsql"/>

6d41b32

jjchiw added 14 commits December 23, 2013 12:16

Accept operator @@ for full text search queries

33cae49

Example: where search_vector @@ to_tsquery('english', @p0)

Find FieldIndex where column name use _

478c00d

CreatedAt (field) == created_at (column name)

Issue #116 - Tests

6f52b65

#116 Tests in NpgsqlTests Project

452d403

#116 fixing references

08a349f

Removing EntityFramework.SqlServer from NpgsTests

#116 Removing App.config

8638212

Removing App.config from Npgsql.EntityFramework

#116 removing the key file from the tests folder

b326be2

#116 Add to app.config <remove invariant="Npgsql"/>

5a07ecb

#116 ConvertToUnderscoreCase if the first comparison fails

1efb8fd

Windows integrated security #if directive for mono

615c4a0

Revert "Windows integrated security #if directive for mono"

87291a9

This reverts commit af0548a.

Fixing rootnamespace

6a5a9d2

Merge remote-tracking branch 'upstream/master'

35238a6

Merge branch 'master' of https://github.com/jjchiw/Npgsql

4e83e97

Conflicts: Npgsql/Npgsql/NpgsqlRowDescription.cs tests/EntityFrameworkTests.cs

kenjiuno mentioned this pull request Jan 2, 2014

Add initial support for ObjectContext.ExecuteFunction #129

Closed

alexrootes mentioned this pull request Mar 25, 2014

'Less Than' prepare failing #193

Closed

jjchiw added a commit to jjchiw/Npgsql that referenced this pull request Mar 31, 2014

npgsql#116 Test Operators @> <@ @@

7d7f41b

Test Operators @> <@ @@

roji force-pushed the master branch 2 times, most recently from 0b35ef0 to c43b6af Compare September 16, 2014 08:50

roji force-pushed the master branch from ea3e18b to 7675866 Compare October 10, 2014 21:58

Emill closed this Aug 9, 2015

Search code, repositories, users, issues, pull requests...

Accept operator @@ to be used in the FullTextSearch. Find FieldIndex where columna name use _ #116

Accept operator @@ to be used in the FullTextSearch. Find FieldIndex where columna name use _ #116

Uh oh!

Conversation

jjchiw commented Dec 6, 2013

Uh oh!

franciscojunior commented Dec 9, 2013

Uh oh!

jjchiw commented Dec 10, 2013

Uh oh!

roji commented Dec 10, 2013

Uh oh!

jjchiw commented Dec 12, 2013

Uh oh!

roji commented Dec 12, 2013

Uh oh!

jjchiw commented Dec 13, 2013

Uh oh!

roji commented Dec 13, 2013

Uh oh!

jjchiw commented Dec 13, 2013

Uh oh!

roji commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 13, 2013

Uh oh!

jjchiw commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 13, 2013

Uh oh!

jjchiw commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 13, 2013

Uh oh!

jjchiw commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 13, 2013

Uh oh!

jjchiw commented Dec 13, 2013

Uh oh!

franciscojunior commented Dec 14, 2013

Uh oh!

roji commented Dec 23, 2013

Uh oh!

jjchiw commented Dec 23, 2013

Uh oh!

roji commented Dec 23, 2013

Uh oh!

glenebob commented Dec 23, 2013

Uh oh!

glenebob commented Mar 29, 2014

Uh oh!

jjchiw commented Mar 31, 2014

Uh oh!

roji commented Aug 9, 2015

Uh oh!

Emill commented Aug 9, 2015

Uh oh!

roji commented Aug 10, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants