Fix: update rows did not work

[davecramer]

if only unique keys were provided. No reason why this should not work
Previously we only retrieved the primary keys. Obviously we could have gone back and done another query for unique keys if there were no primary keys
This would be prohibitively slow. Current solution gets both primary and unique keys.
If there is a primary key we use only the primary keys and remove the unique keys from the list
Otherwise we use the unique keys. Fixes issue #2196

[davecramer]

Thinking about this some more, unique keys allow nulls, we would also need to make sure the column has a non-null constraint on it

[sehrope]

Driving this off of pg_index and including indisunique will also include unique partial indexes. I'm not sure off hand what impact that would have but I'd imagine it might break something if the condition is not met in the updated row.

[davecramer]

I'm starting to question the sanity of using anything but primary indexes. WTF is this Oracle thing ? :)

[davecramer]

we can exclude partial indexes, fairly easy to add that to the query. indpred is not null if it is a partial index

[sehrope]

I think all those APIs are crazy. I mean there's no way to really implement this without a full fledged SQL parser in the driver to figure out what table is being referenced. If it was driven off an API that didn't involve user provided SQL as a string it'd be considerable more sane.

[chalmagr]

There is an additional problem that was pulled in after the change, which is if the table has both (primary key / unique key) then the resulting requirement becomes too broad... e.g. in a table with an id column that is the primary key and a name column (that is unique), and a data column trying to update the resultset for the query "SELECT id, data from myTable" will not allow since the count of P/UK columns is 2 and there is only 1 in the query - this should instead perform a selection of the pk/uk that matches all selected columns (PK can be considered first, so that if PK is met then no need to keep scanning) - as of now, the MetaData retrieval is returning all of them, and using the constraint name to stop processing if all columns in the PK were found in the resultSet... hopefully this makes sense :)

[davecramer]

Ya I'm aware

…

On Mon, Jul 19, 2021, 5:15 PM chalmagr ***@***.***> wrote: There is an additional problem that was pulled in after the change, which is if the table has both (primary key / unique key) then the resulting requirement becomes too broad... e.g. in a table with an id column that is the primary key and a name column (that is unique), and a data column trying to update the resultset for the query "SELECT id, data from myTable" will not allow since the count of P/UK columns is 2 and there is only 1 in the query - this should instead perform a selection of the pk/uk that matches all selected columns (PK can be considered first, so that if PK is met then no need to keep scanning) - as of now, the MetaData retrieval is returning all of them, and using the constraint name to stop processing if all columns in the PK were found in the resultSet... hopefully this makes sense :) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADDH5RLZRJB7DIB22CBMSLTYSIWLANCNFSM5ALWITNQ> .

[chalmagr]

I have made small changes to the code from tag REL42.2.23 - I am currently trying it out... need to read the guidelines for requesting a new PR. not sure what other changes are part of this issue right now or if other issues that I'm unaware of are lingering around... hopefully it will help out :) the response on the issue was quick and I really appreciate it

[davecramer]

Check the other PR I did. I think I have solved the problems

…

On Mon, Jul 19, 2021, 6:36 PM chalmagr ***@***.***> wrote: I have made small changes to the code from tag REL42.2.23 - I am currently trying it out... need to read the guidelines for requesting a new PR. not sure what other changes are part of this issue right now or if other issues that I'm unaware of are lingering around... hopefully it will help out :) the response on the issue was quick and I really appreciate it — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADDH5W3MHEEZP5HBG2CJPDTYSSIRANCNFSM5ALWITNQ> .

[chalmagr]

Added comments to the PR