Bug Fix - Ensure DataQuery::exists() SQL is both valid MySQL and T-SQL

[HARVS1789UK]

The original SQL statement for checking at the DB level if a DataQuery result set has any rows (implemented in #8915) is not valid T-SQL for use with SQL Server.

This pull request aims to tweak that SQL so that it is valid MySQL and T-SQL.

Note that I am not familiar with any other supported DB backends such as SQLite or PostgreSQL so someone with knowledge of these should review this change for compatibility with those please

I have made a more detailed reference to the errors this fixes in #9809

[ScopeyNZ]

This is changing the selected value from a boolean to an integer on PostgreSQL, but it's probably fine as we're checking for 1 in the following condition.

Should work on Postgres (although I haven't tested it) - it only hurts my dev heart a little to see us using ints instead of booleans for a binary value.

[xini]

the return statement can then be simplified I believe?

return (((int) $result) === 1);

[ScopeyNZ]

You have to be careful as PostgreSQL can return 'f' and 't'. (int) 't' === 1 is false.

You could write return $result === 't' || (int) $result === 1 though.

[xini]

@ScopeyNZ we explicitly return 0 or 1 using the CASE WHEN. I don't see how PostgreSQL would make that a 'f' or 't'?

See 02827a6 for where this is already in use like this.

[HARVS1789UK]

@ScopeyNZ (@xini FYI)

Re: "using int rather than boolean for a binary value" - Perhaps I am misinterpreting what you mean, but I have tried replacing 1/0 in the SQL statement with true/false instead but that throws "Invalid column name 'true'" in T-SQL and although it doesn't error in MySQL, the boolean based return values are converted to Int in the resulting output anyway.

Re: making the return statement more succinct, as well as your suggestion, this would also do the trick I suspect:
return (!empty($result) && $result !== 'f');

I won't weigh in on the PostgreSQL comments on 't'/'f' vs 1/0 as I have no experience with PostgreSQL

[ScopeyNZ]

Your PR is fine imo. FYI Postgres has a native boolean type, which is why it's often the odd one out here. TSQL and MySQL don't - but I know that MySQL has some tinyint(1) support which mimics it a little. Can't speak for TSQL.

[sminnee]

I believe this should be fine - note that our Boolean types use a 1-bit integer for PostgreSQL as well to keep a bit more cross-database consistency, so a 1/0 here is fine too IMO.

[HARVS1789UK]

@sminnee does your latest comment support @xini 's proposal to simplify the return statement to return (((int) $result) === 1); then?

2/3 of you have approved these changes, Xini's feedback is still marked as 'requesting changes' of the above, which I am happy to do if we are all in agreement that it is suitable?

Either way if/when that additional change is made and approved, what are the next steps here, all I have an option for is 'Close pull request', should I do that once approval is reached as the creator to confirm it is finished, or is that something one of the three of you do once unanimous approval has been given? (I am not a frequent contributor, bit hazy on the process)

[HARVS1789UK]

Bump ☝️

What needs to be done for this to be merged in?

[ScopeyNZ]

I just need to press the button 🙂