Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118031
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-713-g1f035dc716-fm-20220617.001-g1f035dc7
Mime-Version: 1.0
Message-ID: <ba3a1acf-dde0-41aa-904d-681e04c4a7d2@www.fastmail.com>
In-Reply-To: 
 <CA+kxMuSKYP=favj31kjwkWGC0WWY5rT+RD6H20evQejkr-Nuxw@mail.gmail.com>
References: 
 <CA+kxMuSKYP=favj31kjwkWGC0WWY5rT+RD6H20evQejkr-Nuxw@mail.gmail.com>
Date: Mon, 20 Jun 2022 20:25:46 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] PDO driver specific sub-classes
From: larry@garfieldtech.com ("Larry Garfield")

On Mon, Jun 20, 2022, at 6:01 PM, Dan Ackroyd wrote:
> Hi,
>
> Following previous discussions, here is an RFC to have DB specific
> classes for PDO.
>
> https://wiki.php.net/rfc/pdo_driver_specific_subclasses
>
> cheers
> Dan
> Ack

I'm conceptually in favor of this.  A few comments:

* Any chance I could convince you to move away from the DSN for the factory?  Every DB system I've used has had to do some kind of mangling to build the DSN, but almost never takes its own configuration in the same format, which means there's always some amazingly stupid "parse a URL string to its components, then build up a new string to pass to PDO" step that serves no purpose other than to introduce bugs.  If user/pass are separate args, why can't we just make type and host separate args, too?  Especially with named args now so it's easy to skip or reordr the args.  Please?

* "Should the sub-classes only be avaiable when support for that DB is compiled in?" - I agree, probably not.  Also, "available" is misspelled.

* "Create all DB sub-classes?" - I'd say they all should have subclasses, even if empty.  It's more consistent that way, and you can then also rely on instanceof giving you useful information rather than "well, it's not one of the special ones, so beyond that, NFI."

* Re PQescapeIdentifier - My gut thought is that we should have separate methods for escaping value and identifier in the base class, and then on most DBs they're just identical.  That way it's consistent for anyone building on top of it, because they can rely on escapeValue() and escapeIdentifier() always being The Right Thing To Do without worrying about which DB they're using.

* "This RFC does not propose adding subclasses for any other PDO database not listed above. " - But does it presume that future RFCs may do so?  This could currently be read as "let's not do that" rather than what I think the intent is, which is more "I'm not dealing with that here, talk about that later."  If so, that could be clarified.

--Larry Garfield