UX: Add Crate\PDO\PDOCrateDB as a better export symbol

[amotl]

• UX: Add Crate\PDO\PDOCrateDB as a better export symbol, in order to avoid name collisions with PHP's native PDO class, when imported into the main namespace without an alias.
• Documentation: Add two standalone example programs about insert variants. Presenting valid and fully functional executable example programs to users is always a good thing.

[amotl]

Thoughts

Autoloading, maybe like pdo-via-oci8 is doing it would be nice.

Importing CrateDB PDO's PDO class into the main namespace

Writing only use Crate\PDO\PDO here is a bad idea, and code validators will complain about it, because PDO is already defined in the main namespace - it is PHP's native PDO interface class.

For example, Scrutinizer reports ¹:

This use statement conflicts with another class in this namespace, PDO. Consider defining an alias.

Not importing CrateDB PDO's PDO class

Currently, when not importing Crate\PDO\PDO, new PDO("crate:localhost:4200") will obviously fail like:

PHP Fatal error:  Uncaught PDOException: could not find driver

Proposal

My proposal is to automatically load the driver reference. However, it might not be possible, because regular PDO drivers are written in C/C++.

References

• https://www.reddit.com/r/PHP/comments/uiwlo1/is_it_possible_to_write_a_pdo_driver_in_pure_php/
• https://stackoverflow.com/questions/74665000/register-class-as-pdo-driver

Footnotes

1. Scrutinizer report about insert_basic.php ↩

[amotl]

With d9d97b2, I've added a convenience import symbol Crate\CratePDO, which does not suffer from the import conflict outlined above. Beforehand, I tried a deprecation path, but it proved to be too cumbersome. This variant is much more compact and non-invasive.

[seut]

Interesting findings, didn't know that this results in a conflict even when imported with full namespace. Shouldn't we deprecate Crate\PDO then?

[amotl]

Findings

Interesting findings, didn't know that this results in a conflict even when imported with full namespace.

State of the onion

The conflict is only there when you import the PDO class from another namespace into the main one, where PHP's PDO class is living. Currently, it is only a soft conflict, reported by Scrutinizer and PhpStorm, when importing the symbol. Momentarily, it does not have any runtime impact, but the constraints could get stronger in the future, that's why I was aiming to improve things. It always can be imported without any collision warnings by using an alias like use Crate\PDO\PDO as CratePDO;.

Improvements

It is really only about not using the identical symbol PDO there, in order to be able to import without an alias, like use Crate\CratePDO. We can still talk about the "naming things". Vertica's PDO driver class, for example, is called PDOVertica. Maybe you will prefer that kind of naming, so we could expose that symbol as Crate\PDOCrate or Crate\PDOCrateDB. I think it's only a matter of taste, and I will be happy to hear about your suggestions.

Outlook / conclusion

Shouldn't we deprecate Crate\PDO then?

Having a namespace called Crate\PDO is perfectly fine in general. Also, having a PDO class inside does not do any harm to any symbol outside that namespace. It is only problematic when importing the PDO symbol under the same name into the main namespace.

[seut]

Having a namespace called Crate\PDO is perfectly fine in general. Also, having a PDO class inside does not do any harm to any symbol outside that namespace. It is only problematic when importing the PDO symbol under the same name into the main namespace.

I see, thanks for the clarification. Still I would favour that we only provide one solution in the long term to avoid complexity (maintenance) and user confusion. So I'd vote for deprecating Crate\PDO.

[amotl]

I see, thanks for the clarification. Still I would favour that we only provide one solution in the long term to avoid complexity (maintenance) and user confusion. So I'd vote for deprecating Crate\PDO.

We do need a namespace for the code anyway, and I think the namespace Crate\PDO is perfectly fine and aligned with CrateDB DBAL, which is probably living in Crate\DBAL -- there is absolutely no problem here. The little problem is that the main PDO interface class inside that namespace is called PDO, which collides with the PDO class in PHP's global namespace, when importing it without using an alias.

So, if we agree to rename that class gracefully, we can still discuss the name. I used CratePDO in my initial proposal, but PDOCrateDB would also be possible just renamed it to PDOCrateDB, as it aligns better with the fact that all file names within ¹ start with PDO, and that Vertica's corresponding class is also called PDOVertica.

If we do it that way, I will expand this patch, by:

• Renaming Crate\PDO\PDO and all occurrences within code and documentation to the new and designated class Crate\PDO\PDOCrateDB. I already was half-way through this process, but then decided to consult back with you beforehand.
• Re-adding a backward-compatibility shim class Crate\PDO\PDO, which emits a deprecation warning when used, but will not otherwise break dependent code. This shim class will be removed again with the next release after the upcoming one.

Footnotes

1. https://github.com/crate/crate-pdo/tree/main/src/Crate/PDO ↩

[amotl]

I've just renamed the new proposed class name to Crate\PDO\PDOCrateDB, which is now inside the designated namespace, and aligned with the naming of PDOVertica. I did not swap the code yet, it's still a shim/placeholder, to be able to demonstrate my proposal better. On the final straw, if we agree, the renaming will take place as outlined above.

[seut]

We do need a namespace for the code anyway, and I think the namespace Crate\PDO is perfectly fine and aligned with CrateDB DBAL

My fault, I meant Crate\PDO\PDO, so yes sure, we need to keep the namespace.

If we do it that way, I will expand this patch, by:

• Renaming Crate\PDO\PDO and all occurrences within code and documentation to the new and designated class Crate\PDO\PDOCrateDB. I already was half-way through this process, but then decided to consult back with you beforehand.
• Re-adding a backward-compatibility shim class Crate\PDO\PDO, which emits a deprecation warning when used, but will not otherwise break dependent code. This shim class will be removed again with the next release after the upcoming one.

Sound good, thanks!

[amotl]

Because GH-146 is already stacked on top of this patch, and I don't see any chance to get out of this dilemma, in order to conduct the renaming beforehand, without getting serious merge conflicts later. So, I am integrating those two patches beforehand, and will conduct the renaming on behalf of a subsequent patch.