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

Stop using "IS NOT DISTINCT FROM" #28

Closed
CaucaCG opened this issue May 4, 2016 · 13 comments
Closed

Stop using "IS NOT DISTINCT FROM" #28

CaucaCG opened this issue May 4, 2016 · 13 comments
Assignees
Milestone

Comments

@CaucaCG
Copy link

CaucaCG commented May 4, 2016

Performance issue due to the query generation on update.
It's using "IS NOT DISTINCT FROM" instead of "="
I don't think there's any reason to not use the "=" on not nullable field

@roji
Copy link
Member

roji commented May 4, 2016

Can you provide more detail? Do you have any reason to believe "IS NOT DISTINCT FROM" is slower than "=" in PostgreSQL for non-nullable fields?

@CaucaCG
Copy link
Author

CaucaCG commented May 4, 2016

When I run the query with "IS NOT DISTINCT FROM", it took 600ms, then I replace for "=" and it is only 1ms

@roji
Copy link
Member

roji commented May 4, 2016

Can you please send the two queries which demonstrate this, including a schema?

@CaucaCG
Copy link
Author

CaucaCG commented May 4, 2016

I've made some test with a new table having only two fields, both uuid. If I run the two queries with only few records, it took the same time. When adding 1,000,000 records then I'm starting having performance issue : 10ms vs 87ms.
Then I've added 500,000 more records and it's now 10ms vs 143ms

UPDATE mytable SET parentid='00000000-0000-0000-0000-000000000000' where id iS NOT DISTINCT FROM '015bf336-74a8-4bbd-b4cc-ecdc1fdbe996'
UPDATE mytable SET parentid='00000000-0000-0000-0000-000000000000' where id = '015bf336-74a8-4bbd-b4cc-ecdc1fdbe996'

@roji
Copy link
Member

roji commented May 4, 2016

OK, I'll take a look at this.

@scolemann
Copy link

Another issue is that "IS NOT DISTINCT FROM" will not use an index. Here is a thread from the pgsql-hackers mailing list that discusses it.

http://postgresql.nabble.com/IS-NOT-DISTINCT-FROM-Indexing-td5812296.html

We are running into this issue as well and have update queries timing out because of it. In our case a simple equality would work and would use the index.

@roji roji added this to the 1.0.0-rc2-release2 milestone May 25, 2016
@roji roji self-assigned this May 25, 2016
@roji
Copy link
Member

roji commented May 25, 2016

Thanks for the thread @scolemann, this definitely explains the performance difference @CaucaCG noted above.

I'll make this change soon.

@roji roji changed the title EFCore Update performance issue on Guid Key Stop using "IS NOT DISTINCT FROM" May 25, 2016
@roji
Copy link
Member

roji commented May 25, 2016

Note: I originally used IS NOT DISTINCT FROM as a workaround for dotnet/efcore#3023

@roji
Copy link
Member

roji commented May 25, 2016

So recalling the original problem, this is actually more of an EFCore issue than an Npgsql one - they actually generate good null comparisons for queries but not for updates. That's why I "hacked" things to use IS [NOT] DISTINCT FROM. dotnet/efcore#3023 is about fixing this in EFCore, have just posted there and let's wait to see their response.

I really hope we can avoid implementing doing null comparison logic ourselves here.

@roji roji added the blocked label May 25, 2016
@scolemann
Copy link

@roji thanks for the explanation and sorting this out.

@scolemann
Copy link

Hi @roji,

It looks like the EF team fixed issue 3023 (Update Pipeline: Use C# null semantics for WHERE clauses). Is there any chance you could remove that IS NOT DISTINCT FROM code soon?

Thanks for your help!

@roji roji closed this as completed in 75f10a5 Jun 16, 2016
@roji
Copy link
Member

roji commented Jun 16, 2016

Thanks for telling me @scolemann, that somehow slipped under my radar!

I've removed the IS NOT DISTINCT FROM workaround. Unfortunately it seems there are some outstanding test failures because of the port to rc3, I'll try to solve those ASAP so you can get a working CI version. You can also clone and build yourself, it should be pretty easy.

@roji roji removed the blocked label Jun 16, 2016
@scolemann
Copy link

Awesome, thank you!

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

No branches or pull requests

3 participants