ext-pq API issue: Connection#$encoding

Assigning this can fail. At the moment this can't be easily detected without installing global error handlers (it currently emits an E_NOTICE https://github.com/m6w6/ext-pq/blob/75f3b1b8e2a950e2bce76ad8a68a37472485d3e6/src/php_pqconn.c#L199). Notably, it always fails if you attempt to set it before the connection has been established when connecting asynchronously.

As this is something that could contribute to a security issue if it fails, can we throw an exception instead? A failure to set an encoding anywhere should always be handled - or at least handle-able - IMHO.

May 23 '16 16:05 DaveRandom

Implementing this as property was probably a bad idea, so, should we add a method and deprecate the public property?

May 23 '16 16:05 m6w6

The ideal solution to implement lazy async connection :)

$db = new Connection($dsn); // new instance, not real connection to server

$db->encoding = 'UTF-8'; // if not or bad connect - save value, for deferred execute

yield from $db->exec($sql); // if not or bad connect - async connect and execute deferred query

I understand that it is very difficult, but it will be very convenient to use in userland.

May 23 '16 17:05 ParkFramework

If the main concern is setting the encoding for an async connection, it's probably best to use the client_encoding connection string parameter: http://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNECT-CLIENT-ENCODING

May 23 '16 17:05 m6w6

So I personally don't have any issues with a property assignment throwing. It's something other OO languages do all the time.

However, I will concede that it's not "the PHP way" and it would also be more of a BC issue than deprecating the property and replacing it with a pair of methods, so I'm happy to go with that. I'll check over the rest of the API for anything else that might present a similar problem.

May 24 '16 07:05 DaveRandom

[ ] Throw an exception on invalid property access
(i.e. zend_replace_error_handling in php_pq_object_read_prop/php_pq_object_write_prop)
[ ] Deprecate all connection sensitive properties and introduce getters/setters with exceptions
[ ] Leave everything as it is now
(i.e. E_NOTICE/E_WARNING/E_RECOVERABLE_ERROR (duh!))
[ ] Did I miss something?

Jun 16 '16 06:06 m6w6