Array quoting of placeholders for empty arrays about aura.sql HOT 17 CLOSED

I think that's the correct behavior; there's nothing to quote, after all. What do you suggest?

from aura.sql.

@pmjones The issue is that you are making it transparent to use an array, except for 1 case (which might happen but stay unnoticed when the array is dynamic). For a condition WHERE name IN(?) with an empty array, I would except an empty result set, not a PDOException.
Doctrine DBAL is preparing the statement as IN (NULL), which will produce the expected behavior in case of a positive IN() (being always falsy so rejecting everything). there is still one drawback with this code, when using WHERE a NOT IN(NULL), as NOT IN(NULL) will also be falsy (IN(NULL) will return NULL like for all other SQL operators, and negating NULL is still NULL)

from aura.sql.

So what are you advocating for here?

from aura.sql.

(closed by accident sorry)

from aura.sql.

@pmjones well, the best solution could be to figure a way to have a false condition instead of a NULL one. It may be a bit difficult.

In the meantime, the exception would be really confusing as it would be caused by some modification done by Aura while preparing the query (the user would never find IN() in his code).
The other solution would be to throw the exception earlier when binding the value, where you can give a better exception message explaining that binding an empty array is not supported by Aura.Sql. This way, the user would understand the error.

from aura.sql.

It sounds like the solution easiest to implement would be an exception thrown when quoting an empty array for a bound value. Do you agree with that, or would you prefer something else? (As always, if you have a patch or pull request, I'm happy to review.)

from aura.sql.

(As a side note, it is not the job of the quoting mechanism to build syntactically correct statements; that's up to the developer.)

from aura.sql.

Thank you @stof for the bug report.

Reading all the comments first I thought it will be good to document and add an exception. Looking at how adminer ( by @vrana ) was working when I don't pass any values it worked as @stof mentioning #60 (comment) .

This is a serious case for it is not working as expected though we pass NULL .
We are passing an entirely wrong values and getting wrong values for things like IS NULL should apply . Eg : a field with something can have NULL and may be having values of 1, 2, 3 . In this case IN NULL results should return some values. ( ie also someone will expect )

from aura.sql.

@harikt IN(NULL) always returns NULL (and so is falsy), at least in MySQL. the only operator allowing to check NULL fields is IS, not IN

from aura.sql.

yes that is what I also mean. So doesn't that mean we are altering the query.

from aura.sql.

@stof To reiterate an earlier question: It sounds like the solution easiest to implement would be an exception thrown when quoting an empty array for a bound value. Do you agree with that, or would you prefer something else? (As always, if you have a patch or pull request, I'm happy to review.)

from aura.sql.

@pmjones I think it is fine. It gives a clear error to the user at least. Note that I don't care much about the solution you choose as I don't use Aura.SQL in my projects anyway. I reported the issue because I looked at the package when you announced its v2 and I saw this issue which was faced by Doctrine a few months ago.

from aura.sql.

I appreciate you taking the time to report it, even as a non-user. Thanks!

from aura.sql.

Thank you @stof for reporting it. Your attitude is highly appreciated.

from aura.sql.

-1. We shouldn't even make query for an empty set.

from aura.sql.

@Koc -1 on what ? The latest agreement here is to throw a meaningful exception instead of letting PDO failing with a SQL syntax error, so that users understand their mistake

from aura.sql.

-1 on adding IN(NULL) by default.

from aura.sql.