Query generation in Aura\Sql

[mindplay-dk]

Guys, hold the press for a minute - I think I may have just identified a misplaced feature (or a feature overlap) between Aura\Sql and Aura\Sql_Query.

From the Aura\Sql README:

named placeholders in prepared statements that are bound to array values will be replaced with comma-separated quoted values. This means you can bind an array of values to a placeholder used with an IN (...) condition

This is great, but it has nothing to with "prepared statements" in PDO terms, and it's not strictly an "extension" to PDO - strictly speaking, it's query construction, which is supposed to to be Aura\Sql_Query's domain.

According to the PHP manual, prepared statements:

can be executed multiple times with the same or different parameters. When the query is prepared, the database will analyze, compile and optimize its plan for executing the query

By throwing query construction for arrays into the mix, this is no longer true - as well as:

By using a prepared statement the application avoids repeating the analyze/compile/optimize cycle

This aspect of PDO is now no longer a given in ExtendedPdo - as you know, PDO does not support binding arrays to placeholders, but that is a shortcoming which cannot be addressed by extending the PDO class, since you are then no longer using prepared statements.

While ExtendedPdo arguably "prepares statements", it does not extend the concept of "prepared statements" as defined by PDO. As such, support for binding arrays to statements can't be implemented in userland, because true support for prepared statements is something that requires PDO driver-level support; you don't have any control over the "analyze/compile/optimize cycle", which is an internal feature of PDO.

An array of strings string[] or numbers int[] is a type, and what you're actually doing, is constructing SQL representations of values of those types, for use in SQL queries, which is query construction.

You would not be doing that (and it wouldn't work) for any other types, such as date/time or boolean values. The reason it's even possible at all, without a driver abstraction (which ExtendedPdo does not have, and Sql_Query does) is that the syntax happens to be the same in every supported SQL engine - but that is not a given for any other type.

In other words, this is a query construction concern, not preparation - and I would argue that this has no place in ExtendedPdo; query construction belongs in Aura\Sql_Query, which has a framework to support and handle driver-specific details of SQL generation.

While Aura\Sql_Query is not currently equipped with any driver-specific value-to-SQL or SQL-to-value facilities, this is clearly something you found useful and necessary enough to support for arrays - so a more complete type/value handling facility is probably worth considering?

PDO itself has very bare bones type management - only what is strictly required and/or supported by drivers, e.g. strings, numbers and NULL values. ExtendedPdo tacks on support specifically for arrays of strings, numbers and NULL values, and the end result is a little better, but still isn't by any means a generic framework for all types of values.

Moving support for arrays out of the PDO extension into Sql_Query might be a logical first step towards a more complete (and more general) value formatting facility.

What do you think?

[mindplay-dk]

Note that this could also potentially improve type-safety when binding values with Sql_Query - for example ->where('event.date = ?', date('Y-m-d')) would still be possible and would be bound as a string, but e.g. ->where('event.date = %date', time()) would first pass through a type-check (UNIX timestamp int or DateTime or any registered custom type/function) and then pass through a driver-specific date-formatting function.

Arrays of any type (not just string, int and NULL) could be supported in this manner.

Dibi has something along the lines of this.

If there is interest, I could try prototyping something as proof on concept.

[pmjones]

I am of two minds here.

First, you are (strictly speaking) correct in your assessment. This aspect of ExtendedPdo does break the PDO idea of a prepared statement as a tradeoff against being able to "bind" (really replace) a placeholder to an array value. For what it's worth, the ExtendedPdo uses PDO::bindValue() internally, so there's no expected repetition of "prepared" statements generated by it; you would re-"prepare" it each time with the new values.

Even so, having this at the ExtendedPdo level and not at the query-builder level strikes me as a great boon. There are tons of folks out there who neither need nor want a query builder proper. Mostly what they need is convenience functions on a PDO object, and I see the "binding" of array values of placeholder as a significant convenience. Making these folks use a query builder only so they can get array "binding" seems a bit more overhead than is really needed.

[mindplay-dk]

internally, so there's no expected repetition of "prepared" statements generated by it; you would re-"prepare" it each time with the new values.

This highlights a different (related) problem, I think.

I had some confusion myself early on, as far as the change in semantics - with regular PDO, the Pdo object itself represents the connection and provides factory-methods that create statement objects, and other connection-related functionality. With regular Pdo, the statement objects are responsible for binding - with ExtendedPdo, suddenly the (extended) connection object takes on this responsibility.

Binding is not the responsibility of the connection object, but of the statement object - and you just pinpointed the reason for that: the change in semantics in ExtendedPdo breaks the ability to prepare and reuse statements.

It's a deviation from existing Pdo semantics - the need to depart from existing semantics when extending something, in my experience, is often the result of misunderstanding or disagreeing with the original concept; which, in my experience, often leads to confusion and/or semantic problems.

This is actually a bigger problem, in my opinion - and one that probably ought to be addressed first?

Making these folks use a query builder only so they can get array "binding" seems a bit more overhead than is really needed

Well, you're not "making" them do anything - the same argument works for any piece of software; nobody "has to" use ExtendedPdo to begin with.

Perhaps the biggest problem, on the surface (API) is that, even though the components are technically decoupled and have no symbolic dependency on each other, Aura\Sql_Query currently has an implicit dependency on Aura\Sql, since it relies on it's capability to bind array values - a capability that regular Pdo does not have.

Sql_Query's composer.json "suggests" you use Aura\Sql, but does not in fact work (properly, for arrays) without it. This is a direct consequence of Aura\Sql taking on the query construction responsibility.

I view this as a consequence of two decisions: to depart from fundamental Pdo semantics - and to break separation of concerns in favor of a "boon".

In my opinion, ExtendedPdo should be just that - an extended "better" PDO, not something new or different. The new and different, in my opinion, belongs in Aura\Sql_Query which has entirely new area of responsibility not addressed by Pdo at all.

I think you should view prepared statements not as a means of query construction, but as a means to an end: query optimization.

It is tempting (perhaps natural) to view prepared statements as a primitive or incomplete "query builder", but that isn't it's intended scope at all; prepared statements are a means to an end, which is query optimization, by permitting reuse of prepared statements - it accomplishes that by using tokens and binding values to them, and so it is "complete" in that regard.

Substitution of arrays isn't a missing feature, it's a non-feature - if it isn't supported at the driver-level, it doesn't help you reuse prepared statements, which is all the Pdo API is designed to help you do.

The fact that binding values to tokens also happens to be (part of) a solution to query construction problems, is a mere coincidence. PDO statements are not query builders - they just seem "query builder like" in some regards, because there are similarities, but in my experience an extension yields the best results when it preserves the semantics of the class it extends, and in my opinion this doesn't do that.

It's all relative to opinions and personal experience of course, and I respect that :-)

[mindplay-dk]

Did this give you something to think about?

You should be of two minds, because one could argue in favor of either approach - but I think there is something to be said for staying true to the fundamental concepts you're building upon, as opposed to attempting to mold it into something else, which I think has some major drawbacks.

If you disagree, that's totally fine, and I will stay out of your hair :-)

[pmjones]

Did this give you something to think about?

It did, and does.

Some background: my first attempts were at doing the array-replacement stuff at the PDOStatement level, but as you noted, that was doomed to failure (and did indeed fail).

I am still loathe to place this in query-building. However, maybe there is some middle ground.

How monstrous would it be to have query() etc. return a faked PDO statement that does "type" work internally? Then the faked version could pass on to the real version (which it either wraps or extends) when the actual work is to be done? Hard for me to put it into words; I hope I am putting across the sense of what I mean.

[mindplay-dk]

So you basically want to proxy PDOStatement, right?

Along the lines of:

class ExtendedStatement {
    /** @var PDOStatement */
    protected $statement;

    public function __construct(PDOStatement $statement) {
        $this->statement = $statement;
    }

    public function __get($name) {
        return $this->statement->$name;
    }

    public function __set($name, $value) {
        $this->statement->$name = $value;
    }

    public function __call($name, $params) {
        return call_user_func_array(array($this->statement, $name), $params);
    }

    public function bindParam(...) {
        // array support etc.
    }
}

There's nothing particularly "monstrous" about that - the problem however remains the same as before: Sql_Query is going to have an implicit dependency on this decked-out PDOStatement, since regular PDO connections/statements cannot bind array values.

I don't see any way around that. You're struggling against the fundamental concept upon which you're trying to extend, and I don't think the result is ever going to be simple or elegant.

If you disagree that strongly with the foundation you're building on, here's an entirely different approach: don't extend (or proxy) the existing PDO architecture, at all. Instead, define your own concepts, from the ground up, using PDO as components instead of using them as framework.

Note that there are other small differences in terms of semantics, that I haven't pointed out, which might or might not surface and bother consumers - for example, setAttribute() is supposed to return true/false indicating success; ExtendedPdo can't always do that, because it may not be connected yet. This is just one small difference, but my point is, every change in semantics is going to surface somehow, somewhere, where you notice (or care) or not.

I'm not pro or con either approach. But if you are going to depart from PDO semantics, you would make it a lot easier for someone trying to consume this library, if it has it's own clear-cut semantics entirely, instead of "polluting" well-known PDO semantics. On the other hand, if you are going to pursue the current approach, I think you're going to be happier with the result long-term, if you can find ways to preserve it's existing semantics.

I don't really see a middle ground, but I'm not really a big "middle ground" type of person - I like systems that are conceptually pure and have very well-defined semantics. At the same time, I am also pragmatic - if an existing API doesn't work for me, I have no problems abstracting it away and hiding it entirely behind a new and better API.

In my personal experience, there is no good middle ground - the outcome has to be either a bird or a fish. Flying fish and water birds are fascinating but more complex animals.

I like simple things :-)

[mindplay-dk]

Come to think of it, here's one example of broken semantics in PDO, which is an argument for in favor of departing from the existing API and semantics.

Again, I'm not saying I'm for or against that, but it's a good example, I think.

[mindplay-dk]

Maybe we're looking at this the wrong way.

We're debating whether escaping arrays belongs in Sql or Sql_Query - maybe the answer is both. Perhaps a deliberate slight feature overlap is justifiable and a good thing in this case.

In other words, keep support for binding arrays in Sql, but don't rely on it in Sql_Query - let it perform it's own handling of arrays and (possibly, long term) other types, so that it will truly work with a standard PDO or ExtendedPdo either way.

You don't want coupling between Sql and Sql_Query, and I think that's great - but if you insist on handling arrays in ExtendedPdo, for those who don't want Sql_Query, this is (as far as I can figure) the only way to free Sql_Query from the implicit dependency on ExtendedPdo.

That said though, I have to wonder, what exactly are the practical benefits of ExtendedPdo mimicking native Pdo to a large extent, if ExtendedPdo and Pdo are not truly and fully interchangeable? If you start using ExtendedPdo and rely on it's array capabilities, you are already in a situation where you can't go back to regular Pdo.

So I still have to wonder if extending Pdo even makes sense, for any other purposes than sheer familiarity. (which in itself could be somewhat misleading due to minor semantic and API differences.)

If, for example, you were using a third-party component that requires a (real) Pdo database connection, giving it an ExtendedPdo instead could cause problems, because of (even minor) semantic and API changes. In some ways, it would be safer to aggregate Pdo as a public member of a new Connection class that has it's own semantics - when you need a Pdo connection for some third-party component, you would simply take it from the Connection object, e.g. new Whatever($connection->pdo).

Because ExtendedPdo replaces Pdo, you no longer have access to a "real" Pdo instance, if you need it - you could even get into a situation where you would have to make a second, compatible Pdo connection to the same database, just to satisfy some third-party component's need for a real Pdo instance with accurate Pdo semantics.

Thoughts?

[harikt]

I haven't closely looked all the api of Pdo and ExtendedPdo. From your comments I feel the api is too much different. If the only trouble is on the array eg: IN clauses, I feel they can make a similar method which mimics the ExtendedPdo when needs to be replaced with a real Pdo instance.

[mindplay-dk]

Fundamentally, extends in OOP terms means "is a" - if ExtendedPdo doesn't preserve the semantics of PDO, can you still say that "ExtendedPdo is a PDO"?

If ExtendedPdo can't be said to be a PDO anymore, then I think you should be looking at a "has a" relationship, which means aggregation instead of inheritance.

I have a hard time letting this one go, guys - it seems to me this may be violating a fundamental OOP principle. There is no OOP relationship that can (or should) be described as "kind of works like" - it's either inheritance, in which case it must pass the "is a" test, or otherwise it's something new, for which inheritance is not appropriate. Bird or fish.

I'm still ambivalent as to whether you should use aggregation or inheritance in this design. You make a good case for certain new features and new behaviors, but because those features clash with PDO semantics, you can't have your cake and eat it too - at least not without settling for a flying fish or a water bird.

In my book, simplicity wins, hands down, every time.

The way I see it, you have two options for making this simple:

1. Stick with inheritance, cut certain features and refactor to align with PDO semantics, or
2. Convert to aggregation, keep all your features and define your own semantics.

Again, I don't know which is better, but I want to help out - this library is off to a great start! I think now it's time to figure out where you want it to end up? :-)

[mindplay-dk]

As I'm starting to really work with Sql and Sql_Query, I'm running into another case of broken semantics... In this example, $query is an Insert or Update instance, and $this->db is my ExtendedPdo instance:

    $this->db->bindValues($query->getBindValues());

    $result = $this->db->exec((string) $query);

After the call to bindValues(), the values accumulate as state in the connection object - then, when I call exec(), the accumulated state is then applied to the PDOStatement itself and cleared from the connection object.

The semantics of this transaction is totally broken - you're putting query state in the connection object, which could cause all kinds of horrible problems if you don't strictly and carefully synchronize your calls to e.g. bindValues() and exec(). (or any of the other binding and execution methods.)

What suppose I need to divert between binding some values and actually executing a statement? For example:

    $this->db->bindValues($query->getBindValues());

    $some_value = $this->some_other_method_that_binds_and_executes_a_query();

    $this->db->bindValue('some_value', $some_value);

    $result = $this->db->exec((string) $query);

Because the bind state of queries is accumulated together in the connection object, this will fail.

This is yet another consequence of blurring the responsibilities - your ExtendedPdo class is neither a connection object, nor a statement object; it doesn't strictly stick to the responsibilities you would expect of either of those, so it's a little bit of both.

Have any of you actually used this to build a real application yet? I'm using it heavily on this project, and it's not working as I had hoped. Properly separating concerns/responsibilities in my own software is tricky when the components I'm using aren't properly separated - these SRP issues are leaking badly into my own code...

[mindplay-dk]

By the way, I know the latter example looks "dumb" and "easy to fix" - but you need to consider the fact that not all methods are going to be as obviously named as some_other_method_that_binds_and_executes_a_query - with a name like get_some_important_value(), for example, you don't necessarily know if that value is produced by a query or not. It might be executing a query only under certain conditions - maybe it's caching the value from a previous call, or maybe it only fetches the value under certain conditions. Or worse, maybe today this value is computed and doesn't lead to a query, but tomorrow some other developer adds some logging/auditing (writing to database, calling e.g. execute()) to that method and breaks your code.

The point is, you have to think through all these possibilities - you have to know precisely what may or may not lead to a database query.

Bottom line, this pattern really doesn't work - or it works only under certain very specific conditions. Either way, the root cause of this complexity, is the blurring of the semantics of PDO.

[pmjones]

@mindplay-dk -- that's quite a wall of text you have built up. I want to address your concerns, and perhaps make some remedies as appropriate, but long narratives of this kind are hard to pick apart. I have only so much time. If you can boil your points down to, well, bullet points ;-) I'll be able to respond better.

[mindplay-dk]

Well, to summarize: the library distorts or pollutes (or breaks or collides with) the existing semantics of PDO, and there is no clear separation of concerns, as PDO had.

That's it in a nutshell, really. Everything posted above is just circumstances that demonstrate that point.

[pmjones]

/me nods

If that's the core of the argument, then it seems like the argument is that one should not extend PDO at all, and only wrap it. Adding features makes it even more not-PDO. Is that about right?

[pmjones]

Trying to address individual points here:

With regular Pdo, the statement objects are responsible for binding - with ExtendedPdo, suddenly the (extended) connection object takes on this responsibility.

Yes, since we can't get into the PdoStatement object to add that functionality. If you know of a way to add this to an extended PdoStatement somehow, I'd be all in favor of moving the array value "binding" (really "replacement") there.

[pmjones]

I have to wonder, what exactly are the practical benefits of ExtendedPdo mimicking native Pdo to a large extent ... ?

The idea is that people who are using PDO now can drop in ExtendedPdo as a replacement and have it keep working for them with a minimum of fuss, then begin to use the ExtendedPdo features as they see fit (in particular the fetch*() methods).

[pmjones]

prepared statements are a means to an end, which is query optimization, by permitting reuse of prepared statements

Rarely, if ever, have I reused prepared statements. In addition, they are a means to at least one other end: the prevention of SQL injections via binding values to placeholders.

[pmjones]

If, for example, you were using a third-party component that requires a (real) Pdo database connection, giving it an ExtendedPdo instead could cause problems, because of (even minor) semantic and API changes.

A 3rd-party component would not be using any of the ExtendedPdo features, so I don't see where it would cause problems. However, my imagination is limited; if you could present a brief example where native PDO use would not be amenable to replacement by ExtendedPdo, that might help me out.

[mindplay-dk]

If that's the core of the argument, then it seems like the argument is that one should not extend PDO at all, and only wrap it. Adding features makes it even more not-PDO. Is that about right?

No, you can extend it - like I said earlier, I'm not really por or con either approach.

But when you choose to extend something, you should preserve existing semantics - that doesn't mean you can't add new features, but all existing features must work the way they worked before, for all the reasons explained above.

In a sense, I do think you've chosen by far the rockier road - extension requires deep understanding and close attention to semantics and backwards compatibility, whereas aggregation allows you much more design freedom, without having to worry too much about (100%) backwards compatibility, conceptually nor in terms of public API.

For those times when something does need/expect a native PDO connection, in the case of aggregation, simply make the native objects available:

class Connection
{
    /** @var \Pdo native PDO connection object */
    public $pdo;

    ...
}

class Statement
{
    /** @var PDOStatement native PDO statement object */
    public $sth;

    public function bindValues() {
        // array handling etc here...
    }

    ...
}

if you could present a brief example where native PDO use would not be amenable to replacement by ExtendedPdo, that might help me out

Let me flip that around on you: show me a case (perhaps other than lazy initialization, which can easily be solved in other ways) where replacing PDO with ExtendedPdo is actually an advantage?

If the consumer side is expecting PDO it won't be using any ExtendedPdo features.

And vice-versa, if the consumer side is expecting ExtendedPdo and depends on it's feature, you can't subsitute that for plain PDO.

Or in other words, extension doesn't really accomplish a whole lot in either case - the only real benefit of extending PDO is to preserve the stuff that is already familiar to PDO users, but you could do that and still have the freedom to do other things (without breaking existing semantics) by simply mimicking all or part of PDO itself and making it "familiar enough" that it will be natural and easy to work with for PDO users.

It would probably make more sense for to fork at some point and show you what I'm talking about, rather than spending the same amount of time just explaining it? :-)

[pmjones]

But when you choose to extend something, you should preserve existing semantics - that doesn't mean you can't add new features, but all existing features must work the way they worked before, for all the reasons explained above.

Let's go with this for a moment. What existing feature in PDO works differently in ExtendedPdo? (It would be nice to see some code with PDO that does not work with ExtendedPdo.)

[mindplay-dk]

What existing feature in PDO works differently in ExtendedPdo?

Well, exactly - so why would you use it with a component that only uses PDO features and treats it as a plain PDO?

But for one:

if ($extended_pdo->setAttribute('...')) {
    // will never execute
}

That's a small API break, but nevertheless.

The main problem is not API changes, but changes in terms of semantics and area of responsibility - for example, now I need the actual connection object in order to bind values to a statement, when previously, statements were in charge of that, could be passed around and have values bound to them etc. independently of having access to the connection object.

It completely changes where and when and how you provision the connection and statement objects, and as a consequence, completely changes how you're going to work with these - which is much more critical than whether you can say $connection instanceof PDO or whether it'll survive a type-hint in an argument.

With aggregation, this may seem less convenient, but it's actually simpler - e.g. $my_component->pdo = $extended_pdo with extension vs $my_component->pdo = $connection->pdo with aggregation. If you see the latter as a big loss, then, yeah, extension is the only way to go - but how important is this tiny difference in syntax, really?

What other benefits besides this small difference in syntax (which will most likely rarely affect you to begin with) and the familiarity (which you can get either way) do you get from extension as compared to aggregation?

And don't say "array binding", because we've already established that that creates implicit dependency, and besides, you can have array binding (without overloading any methods or concepts) in a new API with new semantics using aggregation. ;-)

[pmjones]

The main problem is not API changes,

Well, let's start with those first. We can work our way up to other things. I'll look into setAttribute() not behaving the same way -- what else?

p.s. I know you like to write longer narratives, but honestly I can only process so much at one time. Shorter missives are going to be more productive, at least for me. ;-)

[pmjones]

why would you use it with a component that only uses PDO features and treats it as a plain PDO?

I may have mentioned this earlier: the idea is to be able to drop in ExtendedPdo as a direct replacement for PDO so that you can begin using the added features in ExtendedPdo gradually.

[pmjones]

now I need the actual connection object in order to bind values to a statement, when previously, statements were in charge of that, could be passed around and have values bound to them etc. independently of having access to the connection object.

ExtendedPdo continues to work that way. It additionally allows retention of values to be bound to the next query, but the "old" way of working is still available.

[pmjones]

we've already established that [array binding] creates implicit dependency

I don't think _we_ have established that yet. ;-)

[mindplay-dk]

I don't think we have established that yet

Okay, it has been established then, better? ;-)

I may have mentioned this earlier: the idea is to be able to drop in ExtendedPdo as a direct replacement for PDO so that you can begin using the added features in ExtendedPdo gradually.

Familiarity, then - but, as examined, that is not an argument for extension vs aggregation.

ExtendedPdo continues to work that way. It additionally allows retention of values to be bound to the next query, but the "old" way of working is still available.

It's still a change in semantics - the intended case now, is that I bind using the connection object instead of the statement object, which as demonstrated above, completely changes the semantics.

Yeah, I can still bind directly against statements - but suddenly, I don't get the array-support I was promised, and that's confusing.

Bottom line, you've got binding and statement state in two different places now, hence no clear separation of neither the model or the responsibility of binding.

Well, let's start with those first. We can work our way up to other things

I'm fairly convinced you can't patch your way out of this - there is a larger fundamental conceptual issue here, which won't go away with a few API changes, hence the long narratives...

[pmjones]

So, to go back a bit -- there are no other API issues you're aware of?

[pmjones]

Familiarity, then - but, as examined, that is not an argument for extension vs aggregation.

More like "we don't have to change our type hints in order to drop it in, and the existing code will still work." That's not "familiarity" so much as "compatibility."

[pmjones]

I can still bind directly against statements - but suddenly, I don't get the array-support I was promised

The promise, such as it is, it not on PDOStatement, but on ExtendedPdo.

[mindplay-dk]

If the bootstrapping code is the only code that constructs a LegacyApp, what does it matter what this code looks like? It's the only code that needs to be compatible with the main application container's constructor - the application container itself still has a public $pdo same as before, so it remains backwards compatible with all the remaining code in your system.

[mindplay-dk]

Of course, if what you're saying is, you want to be able to drop in ExtendedPdo without ever making any changes to anything, then I see your point... but what would be the point in that? :-)

[pmjones]

Of course, if what you're saying is, you want to be able to drop in ExtendedPdo without ever making any changes to anything, then I see your point... but what would be the point in that? :-)

Yes that is exactly what I am saying. The client code should be able to take an ExtendedPdo and continue working, without any changes, so that the person maintaining that code can work intermittently and piecemeal to begin use the extended functionality. The only part that changes is the setup code, where the maintainer instantiates the shared database connection object as ExtendedPdo instead of PDO.

[mindplay-dk]

The only part that changes is the setup code, where the maintainer instantiates the shared database connection object as ExtendedPdo instead of PDO.

But If that's the only part that changes, you're not using ExtendedPdo. At that stage, why would I care if I'm using PDO or ExtendedPdo? :-)

[pmjones]

But If that's the only part that changes, you're not using ExtendedPdo.

Again, you are exactly right. We want to afford the ability to do incremental, piecemeal changes. First step: swap out PDO for ExtendedPdo, run your tests (even if they're just characterization tests), commit, move on. Change a single method in a single class to use fetchAll(), test, move on. Etc etc.

[mindplay-dk]

The only possible thing I could see you getting out of that, is lazy connection initialization.

[mindplay-dk]

swap out PDO for ExtendedPdo, run your tests

If any of your tests fail, the fun starts here.

If your tests pass, you've accomplished precisely nothing - everything is and works exactly the same as before. It may say new ExtendedPdo in that one line of code, but nowhere are you using any of the new features or gaining anything at all from this change.

I think it's a peculiar use case where I would want to replace a component for no apparent reason.

Personally, I would prefer to add a new component, which doesn't affect anything else, not even potentially - there is no way that even affects any existing tests at all, and the progressive enhancement of each method to use the new component will be just as easy if the new component has a compatible API. (and the PDO API is not big or hard to emulate.)

[pmjones]

If your tests pass, you've accomplished precisely nothing - everything is and works exactly the same as before.

Incorrect -- you have successfully swapped out one piece for another. Now you can start migrating methods.

[pmjones]

Look, man, if you don't like the use-case, I get it. But that is one of the use-cases, and it is unlikely to change.

[mindplay-dk]

You swap out one piece for another. Now you can start migrating methods.

I add a new piece. Now I can start migrating methods.

In real, concrete, practical terms, what's the difference?

Both address your use-case.

[pmjones]

I hate to say this, but I think we are at an impasse. I don't find your arguments on extension-vs-aggregation to be compelling, and I don't think this particular line of discussion is productive. I'm happy to hear (and do what I can to address) other issues unrelated to extension-vs-aggregation; any solutions we negotiate are going to have to be in terms of extension. (I reserve the right to change my mind about extension-vs-aggregation on my own terms. ;-)

[mindplay-dk]

I hate to say this, but I think we are at an impasse

I'm afraid so :-)

I think you have a very fixed idea not just about the use-case or how to address it (which is fine) but precisely what that change should look like in terms of source-code. I don't understand why it's so important that something new (a connection object with new or different capabilities) be able to "stand in" for something old, under the same source-code symbol/name. It is invariably going to have new responsibilities, eventually - otherwise you wouldn't be replacing it to begin with.

Anyhow.

If I had to highlight the most compelling argument from this discussion to leave you with, it's this:

The semantic changes affect where and how you provision connection and statement objects. Where, previously, you may have been passing around PDOStatement objects, and those were sufficient on their own, now you're going to find cases where the statement object is not enough, where you need to also provision the connection object so you can bind array values, or make use of some other new feature. (in practice, I think you're going to find this creates a whole slew of problems and a lot of passing around of the connection and statement objects together.)

If you insist on this approach, as explained earlier, I think you need to at least consider extending the PDOStatement class and correctly delegating responsibilities that (in PDO) are statement-responsibilities - primarily the statement binding state, such that existing PDO semantics are preserved.

Using the extension approach, binding state has no place in the connection object.

You gotta at least give me that? ;-)

[pmjones]

If you insist on this approach, as explained earlier, I think you need to at least consider extending the PDOStatement class and correctly delegating responsibilities that (in PDO) are statement-responsibilities - primarily the statement binding state, such that existing PDO semantics are preserved.

Dude, totally! I tried a couple variations on that and it never worked. If you can make that happen I'd be ecstatic.

[mindplay-dk]

The manual says:

PDO::ATTR_STATEMENT_CLASS: Set user-supplied statement class derived from PDOStatement. Cannot be used with persistent PDO instances. Requires array(string classname, array(mixed constructor_args)).

So yeah, it should be doable. It doesn't state when this option became available though.

[pmjones]

It should be. In my experiments, it was not. But like I said, if you can pull it off, I'd love to see it.

[mindplay-dk]

class Stm extends PDOStatement {}

$pdo = new PDO('mysql:host=localhost;dbname=test', 'root', '');

$pdo->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('Stm'));

var_dump($pdo->prepare('SELECT * FROM test'));

Outputs:

object(Stm)#2 (1) {
  ["queryString"]=>
  string(18) "SELECT * FROM test"
}

Tested in PHP 5.3 and 5.4.

Note that the manual says to pass constructor arguments to setAttribute() which didn't work - e.g. array('Stm', array()) leads to some error message about constructor arguments. Although the manual says that's the format of the arguments, array('classname') is what worked on both 5.3 and 5.4.

[pmjones]

Now try the array "binding" work. :-/

[mindplay-dk]

Ah, I see the problem - yes, "binding" to array values is too late, because the statement has already been prepared, and there's no way to alter the statement once it's been prepared.

Well, as explained previously, existing PDO semantics do not regard arrays a "values" that you can "bind" - so what you're doing is "query construction", which, whether you like it or not, is beyond the scope of PDO, which is the real reason it doesn't fit.

From my point of view, this comes back the real issue, which is you insist on extending something that already has semantics designed for the scope of performing the tasks that PDO performs - which does not include query construction.

If you didn't insist on broadening the scope of PDO, you wouldn't have this problem - keep query construction where it belongs, in your query builder.

Of course, this problem would not exist if you were using aggregation either, because you would be defining your own semantics (in which a "statement" is something that has query construction capabilities) instead of building within existing constraints.

As an (exteremely hacky) work-around, here's what you could do:

class Stm extends PDOStatement {
    public $sth;

    protected function __construct()
    {
        $this->sth = new PDOStatement;
        $this->sth->queryString = $this->queryString;
    }
}

Your statement class is a "vestigial" extension of PDOStatement, only for the sake of passing a type-check, and you would have to internally override everything.

Unfortunately, you can scrap that idea too, because this straight up makes PHP segfault and die.

Of course, you could save yourself all this hacking and trouble by using aggregation, and making your API compatible with PDO and PDOStatement.

I don't know how all of this distortion of semantics is better than just accepting the fact that users will have to do two search and replace operations: PDO to Aura\Sql\Connection and PDOStatement to Aura\Sql\Statement. Doesn't sound like a big issue to me.

Sorry buddy, but aggregation is better - it's simpler, easier, and gives you much more freedom to make choices or define semantics that actually work for what you're doing. You're swimming against the stream :-)

[pmjones]

Well, as explained previously, existing PDO semantics do not regard arrays a "values" that you can "bind" - so what you're doing is "query construction", which, whether you like it or not, is beyond the scope of PDO, which is the real reason it doesn't fit.

This is where we disagree -- what we want is to bind a value to a query. That's not "query construction" any more than binding a string is.

[mindplay-dk]

That's not "query construction" any more than binding a string is.

In PDO terms it is. You may not like it, but that's the way it is.

[pmjones]

Which is why the package extends PDO.

[mindplay-dk]

No, the package extends PDO because you're dead-set on avoiding two search and replace operations to update your type-hints. If you could accept that, aggregation would work - and you would get to define in your own terms what is and is not considered query constructions. Because you're extending PDO, you're going to struggle with it's semantics, because they're not a fit for what you're trying to build.

[pmjones]

I'm not the one in a struggle with the "semantics" -- you are the one who appears to be having trouble. That's fine, you totally get to disagree with the decisions, but going back to aggregation is not going to happen; at least, not as a result of this particular conversation. If there are other issues you have on this package, and your proposed solution is not "use aggregation," then I'm happy to discuss.

[pmjones]

For example, I totally get your concerns about storing bound values for the next query on the ExtendedPdo object. I'm going to re-examine that behavior as a result.

[mindplay-dk]

For example, I totally get your concerns about storing bound values for the next query on the ExtendedPdo object. I'm going to re-examine that behavior as a result.

If extending PDOStatement doesn't work, that's a dead end, isn't it?

I understand your frustration - you "want it all", and I hate to be the bearer of bad news, but sometimes you can't have your cake and eat it too. That's programming :-)

I think you're getting hung up on what is, in reality, a much smaller issue than anything I've brought up, and it's stopping you from making any further progress on this.

Which is totally fine, assuming you're happy and satisfied with what you have - it doesn't meet my standards, but I completely understand that's all subjective and a matter of opinion. I've made my case, and if you still disagree, that's totally fine - I'm not angry or anything, and I won't pursue this any further :-)

[pmjones]

I appreciate your collegial approach, sir, more than you know.

[mindplay-dk]

I have been doing this for long enough to learn that we don't get anywhere by mocking or attacking each other - a lot of developers out there (including my past self!) are incapable of friendly cooperation and respect for other people's opinions because they haven't understood that's what software is: 10% code and 90% opinions :-)

[mindplay-dk]

Also, these discussions are valuable and educational to me, even if I don't get "what I want", and I appreciate you taking the time :-)

[pmjones]

@mindplay-dk I don't think this will make you "happy" but it may make you "less-unhappy":

https://github.com/auraphp/Aura.Sql/tree/nobind

The bind-related methods have been removed, query() and exec() no longer deal with bound values as a result, a new perform() method does the old binding routine in a single step, and the fetch*() methods now use the perform() method. Array replacement still exists but is limited to the perform() method only.