actually fix results_exist for pg

[rabbiveesh]

The current fix for results exists STILL doesn't work on PG, because it doesn't alias the inner subquery (which is necessary at least on PG10, so I have this problem).

This just adds an alias to the table so that pg won't complain.

[coveralls]

Coverage decreased (-0.1%) to 98.599% when pulling 88937f1229b2cc07f36d87241243d7b46a46005e on rabbiveesh:rabbiveesh/fix-exists into 70e81208b35f5011ac1c2b369e81cf9826612f1d on frioux:master.

[frioux]

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

[rabbiveesh]

I could only imagine, but I'll test it out.

On Sat, Nov 16, 2019, 17:54 fREW Schmidt notifications@github.com wrote:

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKX56MTGMUYLUSIU5V3QUAJS3A5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHUNIY#issuecomment-554649251, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKT2TP54D33JUZSU2MLQUAJS3ANCNFSM4JM64FBA .

[frioux]

If you want to dive deeper, this codebase has a framework to run tests against all of Pg, SQL Server, MySQL, and SQLite. Adding a test would ensure your issue stays fixed. Here's one example of where the pg tests are: https://github.com/frioux/DBIx-Class-Helpers/blob/master/t/ResultSet/DateMethods1/pg.t

On Sat, Nov 16, 2019 at 08:12:20AM -0800, Avishai Goldman wrote:

I could only imagine, but I'll test it out.

On Sat, Nov 16, 2019, 17:54 fREW Schmidt notifications@github.com wrote:

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKX56MTGMUYLUSIU5V3QUAJS3A5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHUNIY#issuecomment-554649251, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKT2TP54D33JUZSU2MLQUAJS3ANCNFSM4JM64FBA .

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/frioux/DBIx-Class-Helpers/pull/99#issuecomment-554650784

-- fREW Schmidt https://blog.afoolishmanifesto.com

[rabbiveesh]

Kay, I'll add some backend specific tests. You need them anyways.

On another note, is there any reason that there's no having or join shortcuts? Can I add those to this PR?

On Sat, Nov 16, 2019 at 6:32 PM fREW Schmidt notifications@github.com wrote:

If you want to dive deeper, this codebase has a framework to run tests against all of Pg, SQL Server, MySQL, and SQLite. Adding a test would ensure your issue stays fixed. Here's one example of where the pg tests are:

https://github.com/frioux/DBIx-Class-Helpers/blob/master/t/ResultSet/DateMethods1/pg.t

On Sat, Nov 16, 2019 at 08:12:20AM -0800, Avishai Goldman wrote:

I could only imagine, but I'll test it out.

On Sat, Nov 16, 2019, 17:54 fREW Schmidt notifications@github.com wrote:

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKX56MTGMUYLUSIU5V3QUAJS3A5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHUNIY#issuecomment-554649251 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AFURPKT2TP54D33JUZSU2MLQUAJS3ANCNFSM4JM64FBA

.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub:

https://github.com/frioux/DBIx-Class-Helpers/pull/99#issuecomment-554650784

-- fREW Schmidt https://blog.afoolishmanifesto.com

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKQMKNATTEM4NHFEMJ3QUAOAXA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHVGHA#issuecomment-554652444, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKWZ5BMRLSYT2LOKFSLQUAOAXANCNFSM4JM64FBA .

[frioux]

Sure

-- Sent from a rotary phone rented from Ma Bell

On Sat, Nov 16, 2019, 9:34 AM Avishai Goldman notifications@github.com wrote:

Kay, I'll add some backend specific tests. You need them anyways.

On another note, is there any reason that there's no having or join shortcuts? Can I add those to this PR?

On Sat, Nov 16, 2019 at 6:32 PM fREW Schmidt notifications@github.com wrote:

If you want to dive deeper, this codebase has a framework to run tests against all of Pg, SQL Server, MySQL, and SQLite. Adding a test would ensure your issue stays fixed. Here's one example of where the pg tests are:

https://github.com/frioux/DBIx-Class-Helpers/blob/master/t/ResultSet/DateMethods1/pg.t

On Sat, Nov 16, 2019 at 08:12:20AM -0800, Avishai Goldman wrote:

I could only imagine, but I'll test it out.

On Sat, Nov 16, 2019, 17:54 fREW Schmidt notifications@github.com wrote:

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <

https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKX56MTGMUYLUSIU5V3QUAJS3A5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHUNIY#issuecomment-554649251

,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AFURPKT2TP54D33JUZSU2MLQUAJS3ANCNFSM4JM64FBA

.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub:

https://github.com/frioux/DBIx-Class-Helpers/pull/99#issuecomment-554650784

-- fREW Schmidt https://blog.afoolishmanifesto.com

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKQMKNATTEM4NHFEMJ3QUAOAXA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHVGHA#issuecomment-554652444 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AFURPKWZ5BMRLSYT2LOKFSLQUAOAXANCNFSM4JM64FBA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AAAB7Y4OS2WHL5MNZ53CXQ3QUAVKHA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHWPZI#issuecomment-554657765, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB7Y7BE4KJBIPMVTNI7QLQUAVKHANCNFSM4JM64FBA .

[rabbiveesh]

Hey, sorry. I don't really get how to use the test framework. Is there some more real-time way I can speak to you? Like on IRC?

Dzil keeps complaining that DBICSgen isn't installed, and I see that it's in /inc, but I don't know how to make it work.

On Sat, Nov 16, 2019 at 8:25 PM fREW Schmidt notifications@github.com wrote:

Sure

-- Sent from a rotary phone rented from Ma Bell

On Sat, Nov 16, 2019, 9:34 AM Avishai Goldman notifications@github.com wrote:

Kay, I'll add some backend specific tests. You need them anyways.

On another note, is there any reason that there's no having or join shortcuts? Can I add those to this PR?

On Sat, Nov 16, 2019 at 6:32 PM fREW Schmidt notifications@github.com wrote:

If you want to dive deeper, this codebase has a framework to run tests against all of Pg, SQL Server, MySQL, and SQLite. Adding a test would ensure your issue stays fixed. Here's one example of where the pg tests are:

https://github.com/frioux/DBIx-Class-Helpers/blob/master/t/ResultSet/DateMethods1/pg.t

On Sat, Nov 16, 2019 at 08:12:20AM -0800, Avishai Goldman wrote:

I could only imagine, but I'll test it out.

On Sat, Nov 16, 2019, 17:54 fREW Schmidt notifications@github.com wrote:

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <

https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKX56MTGMUYLUSIU5V3QUAJS3A5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHUNIY#issuecomment-554649251

,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AFURPKT2TP54D33JUZSU2MLQUAJS3ANCNFSM4JM64FBA

.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub:

https://github.com/frioux/DBIx-Class-Helpers/pull/99#issuecomment-554650784

-- fREW Schmidt https://blog.afoolishmanifesto.com

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <

https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKQMKNATTEM4NHFEMJ3QUAOAXA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHVGHA#issuecomment-554652444

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AFURPKWZ5BMRLSYT2LOKFSLQUAOAXANCNFSM4JM64FBA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AAAB7Y4OS2WHL5MNZ53CXQ3QUAVKHA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHWPZI#issuecomment-554657765 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAAB7Y7BE4KJBIPMVTNI7QLQUAVKHANCNFSM4JM64FBA

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKWEREXLFKQ4JFZQAWDQUA3JLA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHXQYI#issuecomment-554661985, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKREDUZQZ7GRGDLN73LQUA3JLANCNFSM4JM64FBA .

[frioux]

I think the CONTRIBUTING file says how

-- Sent from a rotary phone rented from Ma Bell

On Thu, Dec 5, 2019, 12:18 PM Avishai Goldman notifications@github.com wrote:

Hey, sorry. I don't really get how to use the test framework. Is there some more real-time way I can speak to you? Like on IRC?

Dzil keeps complaining that DBICSgen isn't installed, and I see that it's in /inc, but I don't know how to make it work.

On Sat, Nov 16, 2019 at 8:25 PM fREW Schmidt notifications@github.com wrote:

Sure

-- Sent from a rotary phone rented from Ma Bell

On Sat, Nov 16, 2019, 9:34 AM Avishai Goldman notifications@github.com wrote:

Kay, I'll add some backend specific tests. You need them anyways.

On another note, is there any reason that there's no having or join shortcuts? Can I add those to this PR?

On Sat, Nov 16, 2019 at 6:32 PM fREW Schmidt <notifications@github.com

wrote:

If you want to dive deeper, this codebase has a framework to run tests against all of Pg, SQL Server, MySQL, and SQLite. Adding a test would ensure your issue stays fixed. Here's one example of where the pg tests are:

https://github.com/frioux/DBIx-Class-Helpers/blob/master/t/ResultSet/DateMethods1/pg.t

On Sat, Nov 16, 2019 at 08:12:20AM -0800, Avishai Goldman wrote:

I could only imagine, but I'll test it out.

On Sat, Nov 16, 2019, 17:54 fREW Schmidt <notifications@github.com

wrote:

Any idea if this will break other engines? (mysql, sql server, sqlite all work with the current version.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <

https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKX56MTGMUYLUSIU5V3QUAJS3A5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHUNIY#issuecomment-554649251

,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AFURPKT2TP54D33JUZSU2MLQUAJS3ANCNFSM4JM64FBA

.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub:

https://github.com/frioux/DBIx-Class-Helpers/pull/99#issuecomment-554650784

-- fREW Schmidt https://blog.afoolishmanifesto.com

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <

https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKQMKNATTEM4NHFEMJ3QUAOAXA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHVGHA#issuecomment-554652444

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AFURPKWZ5BMRLSYT2LOKFSLQUAOAXANCNFSM4JM64FBA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AAAB7Y4OS2WHL5MNZ53CXQ3QUAVKHA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHWPZI#issuecomment-554657765

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAAB7Y7BE4KJBIPMVTNI7QLQUAVKHANCNFSM4JM64FBA

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKWEREXLFKQ4JFZQAWDQUA3JLA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHXQYI#issuecomment-554661985 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AFURPKREDUZQZ7GRGDLN73LQUA3JLANCNFSM4JM64FBA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AAAB7Y77IGQV2BWCE63G2ITQXFOXTA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGCAHFQ#issuecomment-562299798, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB7Y4NH3ZXVKOLWQIEN7DQXFOXTANCNFSM4JM64FBA .

[ribasushi]

The current fix for results exists STILL doesn't work on PG

@frioux sorry man... Doubly-ridiculous, as I ported the code from a working Pg-based project, but dropped the alias by mistake. Sigh...

[frioux]

If you submit a new pr I can probably release it this weekend

-- Sent from a rotary phone rented from Ma Bell

On Mon, Dec 9, 2019, 2:42 PM Peter Rabbitson notifications@github.com wrote:

@ribasushi commented on this pull request.

In lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm https://github.com/frioux/DBIx-Class-Helpers/pull/99#discussion_r355726141 :

@@ -14,7 +14,7 @@ sub results_exist_as_query { } )->as_query;

• $$reified->[0] = "( SELECT EXISTS $$reified->[0] )";
• $$reified->[0] = "( SELECT EXISTS $$reified->[0] ) as forty_two";

@rabbiveesh https://github.com/rabbiveesh doing this here will in fact break composability in unforseseen ways. Instead you need to do something like: #54 (comment) https://github.com/frioux/DBIx-Class-Helpers/issues/54#issuecomment-563376551

Aliasing a subselect at the point if generation is problematic, as you preclude using multiple such selects in the same "context".

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AAAB7Y2JV2ENFENAQ4V26ELQX3CUDA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOQ5DHY#pullrequestreview-329372063, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB7Y4554VCKSHB2LIVUZLQX3CUDANCNFSM4JM64FBA .

[ribasushi]

@rabbiveesh yep, this should now work. Still needs to be tested against several backends, to make sure we do not repeat the same cycle like last time...

[rabbiveesh]

Well, before i do that, can I get some guidance on how to automate tests against the various back ends? I see that there's some business with Test::Roo, but I'm unfamiliar with it.

On Tue, Dec 10, 2019, 16:08 Peter Rabbitson notifications@github.com wrote:

@ribasushi commented on this pull request.

In lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm https://github.com/frioux/DBIx-Class-Helpers/pull/99#discussion_r356055916 :

@@ -24,11 +24,14 @@ sub results_exist_as_query { sub results_exist { my $self = shift;

• my $query = $self->results_exist_as_query;
• $$query->[0] .= 'AS _existence_subq';

Actually one last-last nit. I'd recommend doing ' AS ... here, to leave an explicit space between the closing ) and the AS. Generally it shouldn't matter, but who knows...

I.e. like in #54 (comment) https://github.com/frioux/DBIx-Class-Helpers/issues/54#issuecomment-563376551

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/pull/99?email_source=notifications&email_token=AFURPKWBQEUPKROSXKWNNEDQX6PHVA5CNFSM4JM64FBKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOUUENY#pullrequestreview-329859639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKTSF6UZBGQMRBSQLXTQX6PHVANCNFSM4JM64FBA .

[ribasushi]

Well, before i do that, can I get some guidance on how to automate tests against the various back ends?

As @frioux said in https://github.com/frioux/DBIx-Class-Helpers/pull/99#issuecomment-562302581: look here: https://github.com/frioux/DBIx-Class-Helpers/blob/master/CONTRIBUTING#L10-L14

[rabbiveesh]

Well, after experimenting I realized that there needs to be a BOATLOAD of work done to get this automatically tested. The current Test::Roo setup is borked b/c it has a weird way to check for connectedness, and therefore doesn't actually test SQLite. Also, the schema files that were generated don't cover enough tables to be usable in the exists test for PG (or the other DBs). So I have to hack around and fix all of that.

Although I can say with 100% confidence that this works on PG and SQLite

[rabbiveesh]

Okay, we're getting there. Just fixing up the testing for Pg to deploy all the tables.

[rabbiveesh]

Okay, that should do it. @frioux please let me know if there's anything else that you need done before this can be merged. The coverage went down b/c I had to add a hack to remove the foreign keys from the table Foo, and that code is only run before the test suite, and thus doesn't count as covered.

[frioux]

@rabbiveesh / @ribasushi This looks great, thanks! Gonna merge and release.

[frioux]

and released